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

[Matthew Simpson via Phabricator via llvm-commits]

mssimpso created this revision.

When combining the indices of GEP chains, we currently wait until the source of the chain has been simplified before simplifying its users. For example, when trying to fold %tmp1 into %tmp2 in the code below, we give up so that we can try to fold %tmp0 into %tmp1 first.

  %tmp0 = getelementptr %pair, %pair* %p, i64 %i
  %tmp1 = getelementptr %pair, %pair* %tmp0, i64 1
  %tmp2 = getelementptr %pair, %pair* %tmp1, i64 0, i32 1

However if the %tmp0 - %tmp1 combine is unsuccessful, we will never actually return to try and simplify %tmp2, even though we can.

This patch causes us to always perform the zero-index simplification useful for this example. With this patch, we will simplify the above code to:

      

  %tmp0 = getelementptr %pair, %pair* %p, i64 %i
  %tmp2 = getelementptr %pair, %pair* %tmp0, i64 1, i32 1


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,16 +1544,36 @@
     if (!shouldMergeGEPs(*cast<GEPOperator>(&GEP), *Src))
       return nullptr;
 
+    SmallVector<Value *, 8> Indices;
+
+    auto createGEPWithIndices = [&]() {
+      return GEP.isInBounds() && Src->isInBounds()
+                 ? GetElementPtrInst::CreateInBounds(
+                       Src->getSourceElementType(), Src->getOperand(0), Indices,
+                       GEP.getName())
+                 : GetElementPtrInst::Create(Src->getSourceElementType(),
+                                             Src->getOperand(0), Indices,
+                                             GEP.getName());
+    };
+
+    // Fold Src into GEP if the first index of GEP is zero. We perform this
+    // simplification regardless of whether Src is also in a GEP chain.
+    if (isa<Constant>(*GEP.idx_begin()) &&
+        cast<Constant>(*GEP.idx_begin())->isNullValue() &&
+        Src->getNumOperands() != 1) {
+      Indices.append(Src->op_begin() + 1, Src->op_end());
+      Indices.append(GEP.idx_begin() + 1, GEP.idx_end());
+      return createGEPWithIndices();
+    }
+
     // 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.
 
-    SmallVector<Value*, 8> Indices;
-
     // Find out whether the last index in the source GEP is a sequential idx.
     bool EndsWithSequential = false;
     for (gep_type_iterator I = gep_type_begin(*Src), E = gep_type_end(*Src);
@@ -1590,22 +1610,8 @@
       Indices.append(Src->op_begin()+1, Src->op_end()-1);
       Indices.push_back(Sum);
       Indices.append(GEP.op_begin()+2, GEP.op_end());
-    } else if (isa<Constant>(*GEP.idx_begin()) &&
-               cast<Constant>(*GEP.idx_begin())->isNullValue() &&
-               Src->getNumOperands() != 1) {
-      // Otherwise we can do the fold if the first index of the GEP is a zero
-      Indices.append(Src->op_begin()+1, Src->op_end());
-      Indices.append(GEP.idx_begin()+1, GEP.idx_end());
+      return createGEPWithIndices();
     }
-
-    if (!Indices.empty())
-      return GEP.isInBounds() && Src->isInBounds()
-                 ? GetElementPtrInst::CreateInBounds(
-                       Src->getSourceElementType(), Src->getOperand(0), Indices,
-                       GEP.getName())
-                 : GetElementPtrInst::Create(Src->getSourceElementType(),
-                                             Src->getOperand(0), Indices,
-                                             GEP.getName());
   }
 
   if (GEP.getNumIndices() == 1) {


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