[PATCH] D30474: [InstCombine] Always Fold GEP chains where the first index is zero

[Matthew Simpson via Phabricator via llvm-commits]

mssimpso updated this revision to Diff 90211.
mssimpso marked an inline comment as done.
mssimpso added a comment.

Addressed feedback from Eli.

- Added a single use condition to the bail out check. If the source GEP has only one use, we now perform the combine instead of bailing out.

This patch didn't hit much in my testing of spec2000, spec2006, and the test-suite (AArch64/Kryo). It generally resulted in a reduction in the static number of instructions in a handful of programs. Performance was noise aside from a ~2% improvement in spec2000/perlbmk.


https://reviews.llvm.org/D30474

Files:
  lib/Transforms/InstCombine/InstructionCombining.cpp
  test/Transforms/InstCombine/getelementptr.ll


Index: test/Transforms/InstCombine/getelementptr.ll
===================================================================
--- test/Transforms/InstCombine/getelementptr.ll
+++ test/Transforms/InstCombine/getelementptr.ll
@@ -931,4 +931,16 @@
   ret i32 addrspace(1)* %x
 }
 
+define i32* @resolve_gep_chains(%pair* %p, i64 %i) {
+; CHECK-LABEL: @resolve_gep_chains(
+; CHECK-NEXT:    [[TMP0:%.*]] = getelementptr %pair, %pair* %p, i64 %i
+; CHECK-NEXT:    [[TMP2:%.*]] = getelementptr %pair, %pair* [[TMP0]], i64 1, i32 1
+; CHECK-NEXT:    ret i32* [[TMP2]]
+;
+  %tmp0 = getelementptr %pair, %pair* %p, i64 %i
+  %tmp1 = getelementptr %pair, %pair* %tmp0, i64 1
+  %tmp2 = getelementptr %pair, %pair* %tmp1, i64 0, i32 1
+  ret i32* %tmp2
+}
+
 ; CHECK: attributes [[NUW]] = { nounwind }
Index: lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- lib/Transforms/InstCombine/InstructionCombining.cpp
+++ lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1544,13 +1544,13 @@
     if (!shouldMergeGEPs(*cast<GEPOperator>(&GEP), *Src))
       return nullptr;
 
-    // Note that if our source is a gep chain itself then we wait for that
-    // chain to be resolved before we perform this transformation.  This
-    // avoids us creating a TON of code in some cases.
-    if (GEPOperator *SrcGEP =
-          dyn_cast<GEPOperator>(Src->getOperand(0)))
-      if (SrcGEP->getNumOperands() == 2 && shouldMergeGEPs(*Src, *SrcGEP))
-        return nullptr;   // Wait until our source is folded to completion.
+    // Note that if our source is a gep chain itself having more than one use
+    // then we wait for that chain to be resolved before we perform this
+    // transformation. This avoids us creating a TON of code in some cases.
+    if (!Src->hasOneUse())
+      if (GEPOperator *SrcGEP = dyn_cast<GEPOperator>(Src->getOperand(0)))
+        if (SrcGEP->getNumOperands() == 2 && shouldMergeGEPs(*Src, *SrcGEP))
+          return nullptr; // Wait until our source is folded to completion.
 
     SmallVector<Value*, 8> Indices;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30474.90211.patch
Type: text/x-patch
Size: 2107 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170301/e822a3e2/attachment.bin>