Fix for #19973

[kvanberendonck]

Summary

Fix for #19973

http://llvm.org/bugs/show_bug.cgi?id=19973

Fails 1 test, which seems to be a test checking for the behaviour I don't think we want.

[kvanberendonck]

Sorry for the whitespace. Originally I had the constant offset check there, but I had to move it because shouldMergeGEP wasn't doing the right thing.

[kvanberendonck]

Fixed junk

[kvanberendonck]

Got to write a regression test and remove the broken test case.

[kvanberendonck]

Now removed the test which was failing (testing for incorrect behaviour) and added 2 new regression test cases -- one of which currently fails (test_gep_canon_2_add). I'm working on getting it right again.

[kvanberendonck]

There may be a constant floating problem in the direction we float constants outlined by the bottom tests. It's hard to pull constants out of stacks of arithmetic because they are moved to the bottom(?). It would be more efficient if they were floated to the top (i.e. closer to the things that use them), maybe.

[chandlerc]

This patch has gone quite stale. I don't really understand why this particular change is the right one, I think it'll need some comments or other clarification in that respect before we accept it... If you end up wanting to continue working on this, try to do that, update the patch, and re-add me so I can take a look.

lib/Transforms/InstCombine/InstructionCombining.cpp

         // normalized.
         if (SO1->getType() != GO1->getType())
           return nullptr;
+          
+        // Is it worth summing the offsets?
+        if (!isa<ConstantInt>(SO1))
+          return nullptr;
+        
         Sum = Builder->CreateAdd(SO1, GO1, PtrOp->getName()+".sum");
       }
 

test/Transforms/InstCombine/getelementptr.ll

   ret void
 }
 
+; Ensure we don't canonicalize chains of GEPs with many unknowns into arithmetic
 define i32* @test7(i32* %I, i64 %C, i64 %D) {
         %A = getelementptr i32* %I, i64 %C
         %B = getelementptr i32* %A, i64 %D
         ret i32* %B
 ; CHECK-LABEL: @test7(
-; CHECK: %A.sum = add i64 %C, %D
-; CHECK: getelementptr i32* %I, i64 %A.sum
+; CHECK: getelementptr i32* %I, i64 %C
+; CHECK: getelementptr i32* %A, i64 %D
 }
 
+; Don't canonicalize away from (%stack - %offset) + idx form (#19973)
+define i32 @test_gep_canon_1(i32* %stack, i64 %offset) {
+        %neg = sub i64 0, %offset
+        %base = getelementptr i32* %stack, i64 %neg
+        %1 = getelementptr i32* %base, i64 1
+        %2 = getelementptr i32* %base, i64 2
+        %A = load i32* %1
+        %B = load i32* %2
+        %ret = add i32 %A, %B
+        ret i32 %ret
+  ; CHECK-LABEL: @test_gep_canon_1(
+  ; CHECK: %neg = sub i64 0, %offset
+  ; CHECK-NEXT: getelementptr i32* %stack, i64 %neg
+  ; CHECK: getelementptr i32* %base, i64 1
+  ; CHECK: getelementptr i32* %base, i64 2
+}
+
+; Fold constant offsets hidden in arithmetic
+define i32* @test_gep_canon_2_add(i32* %A, i32 %B) {
+        %C = add i32 %B, 3
+        %D = getelementptr i32* %A, i32 %C
+        %E = getelementptr i32* %D, i32 4
+        ret i32* %E
+  ; CHECK-LABEL: @test_gep_canon_2_add(
+  ; CHECK: %D = getelementptr i32* %A, i32 %B
+  ; CHECK: getelementptr i32* %D, i32 7
+}
+
 define i8* @test8([10 x i32]* %X) {
         ;; Fold into the cast.
         %A = getelementptr [10 x i32]* %X, i64 0, i64 0