[PATCH] D116587: [ConstantFold] Remove unnecessary bounded index restriction
Nikita Popov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 4 02:59:35 PST 2022
nikic created this revision.
nikic added reviewers: spatel, lebedev.ri, nlopes.
Herald added subscribers: dexonsmith, arphaman, hiraditya.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
The fold for merging a GEP of GEP into a single GEP currently bails if doing so would result in notional overindexing. The justification given in the comment above this check is dangerously incorrect: GEPs with notional overindexing are perfectly fine, and if some code treats them incorrectly, then that code is broken, not the GEP. Such a GEP might legally appear in source IR, so only preventing its creation cannot be sufficient. (The constant folder also ends up canonicalizing the GEP to remove the notional overindexing, but that's neither here nor there.)
This check dates back to https://github.com/llvm/llvm-project/commit/bd4fef4a8939db18f39b108e19097b25e2c7c47a, and as far as I can tell the original issue this was trying to patch around has since been resolved.
https://reviews.llvm.org/D116587
Files:
llvm/lib/IR/ConstantFold.cpp
llvm/test/Transforms/SCCP/apint-bigint2.ll
llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll
Index: llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll
===================================================================
--- llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll
+++ llvm/test/Transforms/SCCP/replace-dereferenceable-ptr-with-undereferenceable.ll
@@ -8,10 +8,10 @@
; CHECK-LABEL: @eq_undereferenceable(
; CHECK-NEXT: entry:
; CHECK-NEXT: store i32 1, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @y, i64 0, i64 0), align 4
-; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32* [[P:%.*]], getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1)
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32* [[P:%.*]], getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 1, i64 0)
; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
; CHECK: if.then:
-; CHECK-NEXT: store i32 2, i32* getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1), align 4
+; CHECK-NEXT: store i32 2, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 1, i64 0), align 4
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
; CHECK-NEXT: [[TMP0:%.*]] = load i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @y, i64 0, i64 0), align 4
@@ -62,13 +62,13 @@
define i1 @eq_undereferenceable_cmp_simp(i32* %p) {
; CHECK-LABEL: @eq_undereferenceable_cmp_simp(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[CMP_0:%.*]] = icmp eq i32* [[P:%.*]], getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1)
+; CHECK-NEXT: [[CMP_0:%.*]] = icmp eq i32* [[P:%.*]], getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 1, i64 0)
; CHECK-NEXT: br i1 [[CMP_0]], label [[IF_THEN:%.*]], label [[IF_END:%.*]]
; CHECK: if.then:
-; CHECK-NEXT: store i32 2, i32* getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1), align 4
+; CHECK-NEXT: store i32 2, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 1, i64 0), align 4
; CHECK-NEXT: ret i1 true
; CHECK: if.end:
-; CHECK-NEXT: [[CMP_2:%.*]] = icmp eq i32* [[P]], getelementptr inbounds (i32, i32* getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 0, i64 0), i64 1)
+; CHECK-NEXT: [[CMP_2:%.*]] = icmp eq i32* [[P]], getelementptr inbounds ([1 x i32], [1 x i32]* @x, i64 1, i64 0)
; CHECK-NEXT: ret i1 [[CMP_2]]
;
entry:
Index: llvm/test/Transforms/SCCP/apint-bigint2.ll
===================================================================
--- llvm/test/Transforms/SCCP/apint-bigint2.ll
+++ llvm/test/Transforms/SCCP/apint-bigint2.ll
@@ -54,7 +54,7 @@
define void @index_too_large() {
; CHECK-LABEL: @index_too_large(
-; CHECK-NEXT: store i101* getelementptr (i101, i101* getelementptr ([6 x i101], [6 x i101]* @Y, i32 0, i32 -1), i101 9224497936761618431), i101** undef, align 8
+; CHECK-NEXT: store i101* getelementptr ([6 x i101], [6 x i101]* @Y, i101 1537416322793603071, i101 4), i101** undef, align 8
; CHECK-NEXT: ret void
;
%ptr1 = getelementptr [6 x i101], [6 x i101]* @Y, i32 0, i32 -1
Index: llvm/lib/IR/ConstantFold.cpp
===================================================================
--- llvm/lib/IR/ConstantFold.cpp
+++ llvm/lib/IR/ConstantFold.cpp
@@ -1998,32 +1998,14 @@
I != E; ++I)
LastI = I;
- // We cannot combine indices if doing so would take us outside of an
- // array or vector. Doing otherwise could trick us if we evaluated such a
- // GEP as part of a load.
- //
- // e.g. Consider if the original GEP was:
- // i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c,
- // i32 0, i32 0, i64 0)
- //
- // If we then tried to offset it by '8' to get to the third element,
- // an i8, we should *not* get:
- // i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c,
- // i32 0, i32 0, i64 8)
- //
- // This GEP tries to index array element '8 which runs out-of-bounds.
- // Subsequent evaluation would get confused and produce erroneous results.
- //
- // The following prohibits such a GEP from being formed by checking to see
- // if the index is in-range with respect to an array.
+ // We can't combine GEPs if the last index is a struct type.
if (!LastI.isSequential())
return nullptr;
+ // We could perform the transform with non-constant index, but prefer leaving
+ // it as GEP of GEP rather than GEP of add for now.
ConstantInt *CI = dyn_cast<ConstantInt>(Idx0);
if (!CI)
return nullptr;
- if (LastI.isBoundedSequential() &&
- !isIndexInRangeOfArrayType(LastI.getSequentialNumElements(), CI))
- return nullptr;
// TODO: This code may be extended to handle vectors as well.
auto *LastIdx = cast<Constant>(GEP->getOperand(GEP->getNumOperands()-1));
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116587.397242.patch
Type: text/x-patch
Size: 4913 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220104/2c682c02/attachment-0001.bin>
More information about the llvm-commits
mailing list