[llvm] c23aefd - [NFC][InstCombine] visitPHINode(): cleanup PHI CSE instruction replacement

[Roman Lebedev via llvm-commits]

Author: Roman Lebedev
Date: 2020-08-31T22:29:39+03:00
New Revision: c23aefd7c38c9413a23fee3664655a10f4a2f41a

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

LOG: [NFC][InstCombine] visitPHINode(): cleanup PHI CSE instruction replacement

As @nikic is pointing out in https://reviews.llvm.org/rGbf21ce7b908e#inline-4647
this must be sufficient otherwise `EliminateDuplicatePHINodes()`
would have hit issues with it already.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
    llvm/test/Transforms/InstCombine/phi-cse.ll
    llvm/test/Transforms/LoopVectorize/reduction.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
index 1e088f3d6cee..539a1f1a128a 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -1439,18 +1439,7 @@ Instruction *InstCombinerImpl::visitPHINode(PHINode &PN) {
       continue;
     // Just use that PHI instead then.
     ++NumPHICSEs;
-    // Note that we can't necessarily just replace the PN with IdenticalPN,
-    // because IdenticalPN might be *after* PN, and PN might have uses
-    // in PHI nodes inbetween the two.
-    // So we need to pick the earliest PHI node as being the canonical one.
-    Instruction *OldPN = &PN;
-    Instruction *NewPN = &IdenticalPN;
-    if (!NewPN->comesBefore(OldPN))
-      std::swap(OldPN, NewPN);
-    replaceInstUsesWith(*OldPN, NewPN);
-    // Also, just to be extra safe, make sure the non-canonical PHI goes away.
-    eraseInstFromFunction(*OldPN);
-    return nullptr; // Signal that thange happened.
+    return replaceInstUsesWith(PN, &IdenticalPN);
   }
 
   // If this is an integer PHI and we know that it has an illegal type, see if

diff  --git a/llvm/test/Transforms/InstCombine/phi-cse.ll b/llvm/test/Transforms/InstCombine/phi-cse.ll
index f369788a5001..5f86a5e893db 100644
--- a/llvm/test/Transforms/InstCombine/phi-cse.ll
+++ b/llvm/test/Transforms/InstCombine/phi-cse.ll
@@ -11,9 +11,9 @@ define void @test0(i32 %v0, i32 %v1, i1 %c, i32* %d0, i32* %d1) {
 ; CHECK:       b1:
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[I0:%.*]] = phi i32 [ [[V0:%.*]], [[B0]] ], [ [[V1:%.*]], [[B1]] ]
-; CHECK-NEXT:    store i32 [[I0]], i32* [[D0:%.*]], align 4
-; CHECK-NEXT:    store i32 [[I0]], i32* [[D1:%.*]], align 4
+; CHECK-NEXT:    [[I1:%.*]] = phi i32 [ [[V0:%.*]], [[B0]] ], [ [[V1:%.*]], [[B1]] ]
+; CHECK-NEXT:    store i32 [[I1]], i32* [[D0:%.*]], align 4
+; CHECK-NEXT:    store i32 [[I1]], i32* [[D1:%.*]], align 4
 ; CHECK-NEXT:    ret void
 ;
 entry:
@@ -229,9 +229,9 @@ define void @test7(i32 %v0, i32 %v1, i16 %v2, i16 %v3, i1 %c, i32* %d0, i32* %d1
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
 ; CHECK-NEXT:    [[IBAD:%.*]] = phi i16 [ [[V2:%.*]], [[B0]] ], [ [[V3:%.*]], [[B1]] ]
-; CHECK-NEXT:    [[I0:%.*]] = phi i32 [ [[V0:%.*]], [[B0]] ], [ [[V1:%.*]], [[B1]] ]
-; CHECK-NEXT:    store i32 [[I0]], i32* [[D0:%.*]], align 4
-; CHECK-NEXT:    store i32 [[I0]], i32* [[D1:%.*]], align 4
+; CHECK-NEXT:    [[I1:%.*]] = phi i32 [ [[V0:%.*]], [[B0]] ], [ [[V1:%.*]], [[B1]] ]
+; CHECK-NEXT:    store i32 [[I1]], i32* [[D0:%.*]], align 4
+; CHECK-NEXT:    store i32 [[I1]], i32* [[D1:%.*]], align 4
 ; CHECK-NEXT:    store i16 [[IBAD]], i16* [[D2:%.*]], align 2
 ; CHECK-NEXT:    ret void
 ;
@@ -262,10 +262,10 @@ define void @test8(i32 %v0, i32 %v1, i16 %v2, i16 %v3, i1 %c, i32* %d0, i32* %d1
 ; CHECK:       b1:
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[I0:%.*]] = phi i32 [ [[V0:%.*]], [[B0]] ], [ [[V1:%.*]], [[B1]] ]
 ; CHECK-NEXT:    [[IBAD:%.*]] = phi i16 [ [[V2:%.*]], [[B0]] ], [ [[V3:%.*]], [[B1]] ]
-; CHECK-NEXT:    store i32 [[I0]], i32* [[D0:%.*]], align 4
-; CHECK-NEXT:    store i32 [[I0]], i32* [[D1:%.*]], align 4
+; CHECK-NEXT:    [[I1:%.*]] = phi i32 [ [[V0:%.*]], [[B0]] ], [ [[V1:%.*]], [[B1]] ]
+; CHECK-NEXT:    store i32 [[I1]], i32* [[D0:%.*]], align 4
+; CHECK-NEXT:    store i32 [[I1]], i32* [[D1:%.*]], align 4
 ; CHECK-NEXT:    store i16 [[IBAD]], i16* [[D2:%.*]], align 2
 ; CHECK-NEXT:    ret void
 ;
@@ -296,10 +296,10 @@ define void @test9(i32 %v0, i32 %v1, i16 %v2, i16 %v3, i1 %c, i32* %d0, i32* %d1
 ; CHECK:       b1:
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[I0:%.*]] = phi i32 [ [[V0:%.*]], [[B0]] ], [ [[V1:%.*]], [[B1]] ]
+; CHECK-NEXT:    [[I1:%.*]] = phi i32 [ [[V0:%.*]], [[B0]] ], [ [[V1:%.*]], [[B1]] ]
 ; CHECK-NEXT:    [[IBAD:%.*]] = phi i16 [ [[V2:%.*]], [[B0]] ], [ [[V3:%.*]], [[B1]] ]
-; CHECK-NEXT:    store i32 [[I0]], i32* [[D0:%.*]], align 4
-; CHECK-NEXT:    store i32 [[I0]], i32* [[D1:%.*]], align 4
+; CHECK-NEXT:    store i32 [[I1]], i32* [[D0:%.*]], align 4
+; CHECK-NEXT:    store i32 [[I1]], i32* [[D1:%.*]], align 4
 ; CHECK-NEXT:    store i16 [[IBAD]], i16* [[D2:%.*]], align 2
 ; CHECK-NEXT:    ret void
 ;

diff  --git a/llvm/test/Transforms/LoopVectorize/reduction.ll b/llvm/test/Transforms/LoopVectorize/reduction.ll
index 33bd3d1fc294..d800351d7eb9 100644
--- a/llvm/test/Transforms/LoopVectorize/reduction.ll
+++ b/llvm/test/Transforms/LoopVectorize/reduction.ll
@@ -503,7 +503,7 @@ exit:
 ;CHECK: shufflevector <4 x i32> %{{.*}}, <4 x i32> undef, <4 x i32> <i32 1, i32 undef, i32 undef, i32 undef>
 ;CHECK: add <4 x i32>
 ;CHECK: extractelement <4 x i32> %{{.*}}, i32 0
-;CHECK: %sum.lcssa = phi i32 [ %[[SCALAR:.*]], %.lr.ph ], [ %[[VECTOR:.*]], %middle.block ]
+;CHECK: %sum.copy = phi i32 [ %[[SCALAR:.*]], %.lr.ph ], [ %[[VECTOR:.*]], %middle.block ]
 ;CHECK: ret i32
 define i32 @reduction_sum_multiuse(i32 %n, i32* noalias nocapture %A, i32* noalias nocapture %B) {
   %1 = icmp sgt i32 %n, 0