[TIR][Driver] Use BindTarget to specify target for FP8 legalization
#16767
Conversation
Lunderberg
left a comment
Lunderberg
left a comment
There was a problem hiding this comment.
Looks good to me, and thank you for the follow-up!
| Array<Pass> mixed_pass_list; | ||
|
|
||
| mixed_pass_list.push_back(tir::transform::FP8ComputeLegalize(target)); | ||
| mixed_pass_list.push_back(tir::transform::BindTarget(target)); |
There was a problem hiding this comment.
Can we add a comment here specifying that the BindTarget should occur first in this sequence, so that later passes can rely on the target attribute being present? (It looks like both VerifyVTCMLimit and LowerVtcmAlloc were implemented more recently than BindTarget, and probably should have been placed after BindTarget at that point.)
There was a problem hiding this comment.
but also, in principle, allows for PrimFuncs with different targets to coexist and be handled correctly by the passes.
Here BindTarget is applying the target to all functions in the mixed IRModule, no?
There was a problem hiding this comment.
Yeah that's what it does. In principle, we could set the attributes differently.
There was a problem hiding this comment.
Slight nitpick: All functions that do not already have the target attribute. A function could already have the target attribute, such as defining a module that contains kernels for multiple different targets.
|
@csullivan @JosephTheOctonaut FYI on this change, as it relates to your work on FP8. I don't expect it to impact your work, but wanting to make sure you're aware of it. |
…apache#16767) * Do not pass target explicitly to FP8 legalization, use BindTarget instead * Lint: Remove unused import * Add comment on pass ordering
4 participants
This PR cleans up the interface for the FP8 legalization passes by having them use the target attributes that are added by the
BindTargetpass. This not only removes the need to pass the target as a parameter to those passes but also, in principle, allows for PrimFuncs with different targets to coexist and be handled correctly by the passes. Thanks to @Lunderberg for giving the suggestion.