[llvm] 940f892 - [InstCombine] Do not modify GEP in place

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2024-09-13T10:04:39+02:00
New Revision: 940f89255e4a3982d94dad57837e8e658092af78

URL: https://github.com/llvm/llvm-project/commit/940f89255e4a3982d94dad57837e8e658092af78
DIFF: https://github.com/llvm/llvm-project/commit/940f89255e4a3982d94dad57837e8e658092af78.diff

LOG: [InstCombine] Do not modify GEP in place

This was modifying the GEP in place, with code to adjust the
inbounds flag. This was correct at the time, but now fails to
account for other GEP flags like nuw, leading to miscompilations.

Remove the special case, and always create a new GEP instruction.
Logic for preserving nuw in the cases where it is valid will be
added in a followup patch.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
    llvm/test/Transforms/InstCombine/getelementptr.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 8195e0539305cc..45ae6d342742af 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2508,13 +2508,6 @@ Instruction *InstCombinerImpl::visitGEPOfGEP(GetElementPtrInst &GEP,
     if (Sum == nullptr)
       return nullptr;
 
-    // Update the GEP in place if possible.
-    if (Src->getNumOperands() == 2) {
-      GEP.setIsInBounds(isMergedGEPInBounds(*Src, *cast<GEPOperator>(&GEP)));
-      replaceOperand(GEP, 0, Src->getOperand(0));
-      replaceOperand(GEP, 1, Sum);
-      return &GEP;
-    }
     Indices.append(Src->op_begin()+1, Src->op_end()-1);
     Indices.push_back(Sum);
     Indices.append(GEP.op_begin()+2, GEP.op_end());

diff  --git a/llvm/test/Transforms/InstCombine/getelementptr.ll b/llvm/test/Transforms/InstCombine/getelementptr.ll
index c805a64d5cd070..8607e58f6fa585 100644
--- a/llvm/test/Transforms/InstCombine/getelementptr.ll
+++ b/llvm/test/Transforms/InstCombine/getelementptr.ll
@@ -1905,5 +1905,17 @@ define ptr @gep_of_ashr_fail_not_divisible(ptr %p, i64 %x) {
   ret ptr %r
 }
 
+define ptr @gep_merge_not_nuw(ptr %p, i64 %idx) {
+; CHECK-LABEL: @gep_merge_not_nuw(
+; CHECK-NEXT:    [[GEP:%.*]] = getelementptr inbounds i8, ptr [[P:%.*]], i64 -1
+; CHECK-NEXT:    ret ptr [[GEP]]
+;
+  %idx.neg = sub i64 0, %idx
+  %add = add i64 %idx, -1
+  %gep1 = getelementptr inbounds i8, ptr %p, i64 %idx.neg
+  %gep = getelementptr inbounds nuw i8, ptr %gep1, i64 %add
+  ret ptr %gep
+}
+
 
 !0 = !{!"branch_weights", i32 2, i32 10}