From ebacf44320b5aa59efa03cd0ae90935fc4b953fd Mon Sep 17 00:00:00 2001
From: Silviu Baranga <silviu.baranga@arm.com>
Date: Mon, 26 Oct 2015 10:25:05 +0000
Subject: [PATCH] [InstCombine] Teach instcombine not to create extra PHI nodes
 when folding GEPs

Summary:
InstCombine tries to transform GEP(PHI(GEP1, GEP2, ..)) into GEP(GEP(PHI(...))
when possible. However, this may leave the old PHI node around. Even if we
do end up folding the GEPs, having an extra PHI node might not be beneficial.

This change makes the transformation more conservative. We now only do this if
the PHI has only one use, and can therefore be removed after the transformation.

Reviewers: jmolloy, majnemer

Subscribers: mcrosier, mssimpso, llvm-commits

Differential Revision: http://reviews.llvm.org/D13887

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@251281 91177308-0d34-0410-b5e6-96231b3b80d8
---
 .../InstCombine/InstructionCombining.cpp      |  7 ++-
 test/Transforms/InstCombine/gepphigep.ll      | 50 +++++++++++++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/lib/Transforms/InstCombine/InstructionCombining.cpp b/lib/Transforms/InstCombine/InstructionCombining.cpp
index 69a2661f031..b51ec291f91 100644
--- a/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1453,8 +1453,13 @@ Instruction *InstCombiner::visitGetElementPtrInst(GetElementPtrInst &GEP) {
       }
     }
 
-    GetElementPtrInst *NewGEP = cast<GetElementPtrInst>(Op1->clone());
+    // If not all GEPs are identical we'll have to create a new PHI node.
+    // Check that the old PHI node has only one use so that it will get
+    // removed.
+    if (DI != -1 && !PN->hasOneUse())
+      return nullptr;
 
+    GetElementPtrInst *NewGEP = cast<GetElementPtrInst>(Op1->clone());
     if (DI == -1) {
       // All the GEPs feeding the PHI are identical. Clone one down into our
       // BB so that it can be merged with the current GEP.
diff --git a/test/Transforms/InstCombine/gepphigep.ll b/test/Transforms/InstCombine/gepphigep.ll
index b98ea4cd115..cc90d714be7 100644
--- a/test/Transforms/InstCombine/gepphigep.ll
+++ b/test/Transforms/InstCombine/gepphigep.ll
@@ -134,3 +134,53 @@ exit:
 ; CHECK: getelementptr{{.*}}i64 1
 ; CHECK: exit:
 }
+
+@.str.4 = external unnamed_addr constant [100 x i8], align 1
+
+; Instcombine shouldn't add new PHI nodes while folding GEPs if that will leave
+; old PHI nodes behind as this is not clearly beneficial.
+; CHECK-LABEL: @test5(
+define void @test5(i16 *%idx, i8 **%in) #0 {
+entry:
+  %0 = load i8*, i8** %in
+  %incdec.ptr = getelementptr inbounds i8, i8* %0, i32 1
+  %1 = load i8, i8* %incdec.ptr, align 1
+  %cmp23 = icmp eq i8 %1, 54
+  br i1 %cmp23, label %while.cond, label %if.then.25
+
+if.then.25:
+  call void @g(i8* getelementptr inbounds ([100 x i8], [100 x i8]* @.str.4, i32 0, i32 0))
+  br label %while.cond
+
+while.cond:
+; CHECK-LABEL: while.cond
+; CHECK-NOT: phi i8* [ %0, %entry ], [ %Ptr, %while.body ], [ %0, %if.then.25 ]
+  %Ptr = phi i8* [ %incdec.ptr, %entry ], [ %incdec.ptr32, %while.body], [%incdec.ptr, %if.then.25 ]
+  %2 = load i8, i8* %Ptr
+  %and = and i8 %2, 64
+  %lnot = icmp eq i8 %and, 0
+  br i1 %lnot, label %while.body, label %while.cond.33
+
+while.body:
+  %incdec.ptr32 = getelementptr inbounds i8, i8* %Ptr, i32 1
+  br label %while.cond
+
+while.cond.33:
+  %incdec.ptr34 = getelementptr inbounds i8, i8* %Ptr, i32 1
+  br label %while.cond.57
+
+while.cond.57:
+  %3 = load i8, i8* %incdec.ptr34, align 1
+  %conv59 = zext i8 %3 to i32
+  %arrayidx61 = getelementptr inbounds i16, i16* %idx, i32 %conv59
+  %4 = load i16, i16* %arrayidx61, align 2
+  %and63 = and i16 %4, 2048
+  %tobool64 = icmp eq i16 %and63, 0
+  br i1 %tobool64, label %while.cond.73, label %while.cond.57
+
+while.cond.73:
+  br label %while.cond.73
+
+}
+
+declare void @g(i8*)
-- 
2.34.1