-
Notifications
You must be signed in to change notification settings - Fork 790
[SYCLLowerIR][GlobalOffset] Fix collection order of load's defs #20428
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
Conversation
This PR fixes two issues in previous logic: * phi user of @llvm.amdgcn.implicit.offset call is not handled. * post-order collecting load's defs is wrong, producing wrong order in `PtrUses`.
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.
LGTM, few nits to consider.
@@ -0,0 +1,45 @@ | |||
; RUN: opt -bugpoint-enable-legacy-pm -globaloffset %s -S -o - | FileCheck %s | |||
|
|||
; Check that phi is correctly handled in load's defs collection. |
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.
nit:
would it make sense to add test, where result of load is used? What would happen with those uses?
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.
nit: would it make sense to add test, where result of load is used? What would happen with those uses?
done, updated the test to have detailed check, thanks
@intel/llvm-gatekeepers please merge, thanks |
This PR fixes two issues in previous logic:
PtrUses
.