-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Bugfix][TIR] Handle AttrStmt of upcoming tir.Var in ConvertSSA #16682
Conversation
In some cases, an `AttrStmt` may legally refer to a TIR variable that hasn't yet been defined. For example, the `"pragma_parallel_launch_point"` attribute, which annotates a variable that is about to occur in a ForNode. Prior to this commit, `ConvertSSA` treated the `AttrStmt` as the usage of a variable, followed by a nested definition to be de-duplicated. This resulted in the output `AttrStmt` containing a reference to an undefined variable. This commit updates `ConvertSSA` to handle this case. If an `AttrStmt` refers to a not-yet-defined variable, the body is visited before marking it as defined. This implementation may be simplified in the future by moving "pragma_parallel_launch_point" to be an annotation on the `ForNode`, rather than an `AttrStmt`.
794e51c
to
f480fa4
Compare
@slyubomirsky This should unblock #16682, as it resolves the issue found in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the change. Would this also work for attributes that do act as points of definition?
It should, yes. This behavior is tested in |
…he#16682) In some cases, an `AttrStmt` may legally refer to a TIR variable that hasn't yet been defined. For example, the `"pragma_parallel_launch_point"` attribute, which annotates a variable that is about to occur in a ForNode. Prior to this commit, `ConvertSSA` treated the `AttrStmt` as the usage of a variable, followed by a nested definition to be de-duplicated. This resulted in the output `AttrStmt` containing a reference to an undefined variable. This commit updates `ConvertSSA` to handle this case. If an `AttrStmt` refers to a not-yet-defined variable, the body is visited before marking it as defined. This implementation may be simplified in the future by moving "pragma_parallel_launch_point" to be an annotation on the `ForNode`, rather than an `AttrStmt`.
…he#16682) In some cases, an `AttrStmt` may legally refer to a TIR variable that hasn't yet been defined. For example, the `"pragma_parallel_launch_point"` attribute, which annotates a variable that is about to occur in a ForNode. Prior to this commit, `ConvertSSA` treated the `AttrStmt` as the usage of a variable, followed by a nested definition to be de-duplicated. This resulted in the output `AttrStmt` containing a reference to an undefined variable. This commit updates `ConvertSSA` to handle this case. If an `AttrStmt` refers to a not-yet-defined variable, the body is visited before marking it as defined. This implementation may be simplified in the future by moving "pragma_parallel_launch_point" to be an annotation on the `ForNode`, rather than an `AttrStmt`.
In some cases, an
AttrStmt
may legally refer to a TIR variable that hasn't yet been defined. For example, the"pragma_parallel_launch_point"
attribute, which annotates a variable that is about to occur in a ForNode. Prior to this commit,ConvertSSA
treated theAttrStmt
as the usage of a variable, followed by a nested definition to be de-duplicated. This resulted in the outputAttrStmt
containing a reference to an undefined variable.This commit updates
ConvertSSA
to handle this case. If anAttrStmt
refers to a not-yet-defined variable, the body is visited before marking it as defined.This implementation may be simplified in the future by moving "pragma_parallel_launch_point" to be an annotation on the
ForNode
, rather than anAttrStmt
.