-
-
Notifications
You must be signed in to change notification settings - Fork 268
Emit getelementptr inbounds nuw flag where possible #4936
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
base: master
Are you sure you want to change the base?
Conversation
tests/codegen/inbounds.d
Outdated
return *++p; | ||
} | ||
|
||
// Add offset to pointer | ||
// CHECK-LABEL: @foo6 | ||
int foo6(int* p, int i) { | ||
// CHECK: getelementptr inbounds | ||
// CHECK: getelementptr inbounds{{( nuw)?}} |
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.
Is this correct? For p == 0xffff_ffff_ffff_0000
and i == 0x1_0000
, there would be overflow (in unsigned sense).
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.
Maybe a better example is where i
is negative.
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.
"For inbounds
all rules of the nusw
keyword apply"
"A corollary is that if the getelementptr
is both nusw
and nuw
, the offset must be non-negative"
So I think this PR is not correct when i
can be negative (as in this test case).
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.
You're right, but we can't guarantee inbounds for any of the pointer operations also, can we?
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.
You're right, but we can't guarantee inbounds for any of the pointer operations also, can we?
I think we can, by language spec, because out of bounds would be UB (as specced).
Please be very thorough in proving that this is correct. I fear that if this is slightly incorrect, that it can lead to hard to debug miscompiles at high optimization levels. |
LLVM 20 introduces new getelementptr flag
nuw
.inbounds + nuw
implies that the offset is non-negative, so this could be useful for eliminating bounds/overflow check.see also: https://discourse.llvm.org/t/rfc-add-nusw-and-nuw-flags-for-getelementptr/78672