[llvm] c295c6f - Revert "[InstCombine] PHI-of-extractvalues -> extractvalue-of-PHI, aka invokes are bad"

Roman Lebedev via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 25 23:24:45 PDT 2020


Author: Roman Lebedev
Date: 2020-08-26T09:23:22+03:00
New Revision: c295c6f2c04e771f44322e5dfdd681a5ace300e5

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

LOG: Revert "[InstCombine] PHI-of-extractvalues -> extractvalue-of-PHI, aka invokes are bad"

This reverts commit fcb51d8c2460faa23b71e06abb7e826243887dd6.

As buildbots report, there's apparently some missing check to ensure
that the types of incoming values match the type of PHI.
Let's revert for a moment.

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineInternal.h
    llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
    llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll
    llvm/test/Transforms/InstCombine/phi-of-extractvalues.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index 86b0bfe24d28..e5fe99367b7b 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -618,7 +618,6 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
   Instruction *foldPHIArgOpIntoPHI(PHINode &PN);
   Instruction *foldPHIArgBinOpIntoPHI(PHINode &PN);
   Instruction *foldPHIArgInsertValueInstructionIntoPHI(PHINode &PN);
-  Instruction *foldPHIArgExtractValueInstructionIntoPHI(PHINode &PN);
   Instruction *foldPHIArgGEPIntoPHI(PHINode &PN);
   Instruction *foldPHIArgLoadIntoPHI(PHINode &PN);
   Instruction *foldPHIArgZextsIntoPHI(PHINode &PN);

diff  --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
index 23a85b9e4f1d..269b4d550e43 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -32,8 +32,6 @@ MaxNumPhis("instcombine-max-num-phis", cl::init(512),
 
 STATISTIC(NumPHIsOfInsertValues,
           "Number of phi-of-insertvalue turned into insertvalue-of-phis");
-STATISTIC(NumPHIsOfExtractValues,
-          "Number of phi-of-extractvalue turned into extractvalue-of-phi");
 
 /// The PHI arguments will be folded into a single operation with a PHI node
 /// as input. The debug location of the single operation will be the merged
@@ -338,41 +336,6 @@ InstCombinerImpl::foldPHIArgInsertValueInstructionIntoPHI(PHINode &PN) {
   return NewIVI;
 }
 
-/// If we have something like phi [extractvalue(a,0), extractvalue(b,0)],
-/// turn this into a phi[a,b] and a single extractvalue.
-Instruction *
-InstCombinerImpl::foldPHIArgExtractValueInstructionIntoPHI(PHINode &PN) {
-  auto *FirstEVI = cast<ExtractValueInst>(PN.getIncomingValue(0));
-
-  // Scan to see if all operands are `extractvalue`'s with the same indicies,
-  // and all have a single use.
-  for (unsigned i = 1; i != PN.getNumIncomingValues(); ++i) {
-    auto *I = dyn_cast<ExtractValueInst>(PN.getIncomingValue(i));
-    if (!I || !I->hasOneUse() || I->getIndices() != FirstEVI->getIndices())
-      return nullptr;
-  }
-
-  // Create a new PHI node to receive the values the aggregate operand has
-  // in each incoming basic block.
-  auto *NewAggregateOperand = PHINode::Create(
-      FirstEVI->getAggregateOperand()->getType(), PN.getNumIncomingValues(),
-      FirstEVI->getAggregateOperand()->getName() + ".pn");
-  // And populate the PHI with said values.
-  for (auto Incoming : zip(PN.blocks(), PN.incoming_values()))
-    NewAggregateOperand->addIncoming(
-        cast<ExtractValueInst>(std::get<1>(Incoming))->getAggregateOperand(),
-        std::get<0>(Incoming));
-  InsertNewInstBefore(NewAggregateOperand, PN);
-
-  // And finally, create `extractvalue` over the newly-formed PHI nodes.
-  auto *NewEVI = ExtractValueInst::Create(NewAggregateOperand,
-                                          FirstEVI->getIndices(), PN.getName());
-
-  PHIArgMergedDebugLoc(NewEVI, PN);
-  ++NumPHIsOfExtractValues;
-  return NewEVI;
-}
-
 /// If we have something like phi [add (a,b), add(a,c)] and if a/b/c and the
 /// adds all have a single use, turn this into a phi and a single binop.
 Instruction *InstCombinerImpl::foldPHIArgBinOpIntoPHI(PHINode &PN) {
@@ -825,8 +788,6 @@ Instruction *InstCombinerImpl::foldPHIArgOpIntoPHI(PHINode &PN) {
     return foldPHIArgLoadIntoPHI(PN);
   if (isa<InsertValueInst>(FirstInst))
     return foldPHIArgInsertValueInstructionIntoPHI(PN);
-  if (isa<ExtractValueInst>(FirstInst))
-    return foldPHIArgExtractValueInstructionIntoPHI(PN);
 
   // Scan the instruction, looking for input operations that can be folded away.
   // If all input operands to the phi are the same instruction (e.g. a cast from

diff  --git a/llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll b/llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll
index 939126bd2e34..b97b40827ccd 100644
--- a/llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll
+++ b/llvm/test/Transforms/InstCombine/phi-aware-aggregate-reconstruction.ll
@@ -24,14 +24,9 @@ define { i32, i32 } @test0({ i32, i32 } %agg_left, { i32, i32 } %agg_right, i1 %
 ; CHECK-NEXT:    call void @bar()
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[AGG_LEFT_PN:%.*]] = phi { i32, i32 } [ [[AGG_LEFT:%.*]], [[LEFT]] ], [ [[AGG_RIGHT:%.*]], [[RIGHT]] ]
-; CHECK-NEXT:    [[AGG_LEFT_PN1:%.*]] = phi { i32, i32 } [ [[AGG_LEFT]], [[LEFT]] ], [ [[AGG_RIGHT]], [[RIGHT]] ]
-; CHECK-NEXT:    [[I6:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT_PN1]], 1
-; CHECK-NEXT:    [[I5:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT_PN]], 0
+; CHECK-NEXT:    [[I8_MERGED:%.*]] = phi { i32, i32 } [ [[AGG_RIGHT:%.*]], [[RIGHT]] ], [ [[AGG_LEFT:%.*]], [[LEFT]] ]
 ; CHECK-NEXT:    call void @baz()
-; CHECK-NEXT:    [[I7:%.*]] = insertvalue { i32, i32 } undef, i32 [[I5]], 0
-; CHECK-NEXT:    [[I8:%.*]] = insertvalue { i32, i32 } [[I7]], i32 [[I6]], 1
-; CHECK-NEXT:    ret { i32, i32 } [[I8]]
+; CHECK-NEXT:    ret { i32, i32 } [[I8_MERGED]]
 ;
 entry:
   br i1 %c, label %left, label %right
@@ -63,16 +58,18 @@ define { i32, i32 } @negative_test1({ i32, i32 } %agg_left, { i32, i32 } %agg_ri
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[LEFT:%.*]], label [[RIGHT:%.*]]
 ; CHECK:       left:
+; CHECK-NEXT:    [[I4:%.*]] = extractvalue { i32, i32 } [[AGG_RIGHT:%.*]], 1
+; CHECK-NEXT:    [[I0:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT:%.*]], 0
 ; CHECK-NEXT:    call void @foo()
 ; CHECK-NEXT:    br label [[END:%.*]]
 ; CHECK:       right:
+; CHECK-NEXT:    [[I3:%.*]] = extractvalue { i32, i32 } [[AGG_RIGHT]], 0
+; CHECK-NEXT:    [[I2:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT]], 1
 ; CHECK-NEXT:    call void @bar()
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[AGG_LEFT_PN:%.*]] = phi { i32, i32 } [ [[AGG_LEFT:%.*]], [[LEFT]] ], [ [[AGG_RIGHT:%.*]], [[RIGHT]] ]
-; CHECK-NEXT:    [[AGG_RIGHT_PN:%.*]] = phi { i32, i32 } [ [[AGG_RIGHT]], [[LEFT]] ], [ [[AGG_LEFT]], [[RIGHT]] ]
-; CHECK-NEXT:    [[I6:%.*]] = extractvalue { i32, i32 } [[AGG_RIGHT_PN]], 1
-; CHECK-NEXT:    [[I5:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT_PN]], 0
+; CHECK-NEXT:    [[I5:%.*]] = phi i32 [ [[I0]], [[LEFT]] ], [ [[I3]], [[RIGHT]] ]
+; CHECK-NEXT:    [[I6:%.*]] = phi i32 [ [[I4]], [[LEFT]] ], [ [[I2]], [[RIGHT]] ]
 ; CHECK-NEXT:    call void @baz()
 ; CHECK-NEXT:    [[I7:%.*]] = insertvalue { i32, i32 } undef, i32 [[I5]], 0
 ; CHECK-NEXT:    [[I8:%.*]] = insertvalue { i32, i32 } [[I7]], i32 [[I6]], 1
@@ -157,20 +154,24 @@ define { i32, i32 } @test3({ i32, i32 } %agg_00, { i32, i32 } %agg_01, { i32, i3
 ; CHECK:       bb0.dispatch:
 ; CHECK-NEXT:    br i1 [[C1:%.*]], label [[BB00:%.*]], label [[BB01:%.*]]
 ; CHECK:       bb00:
+; CHECK-NEXT:    [[I0:%.*]] = extractvalue { i32, i32 } [[AGG_00:%.*]], 0
+; CHECK-NEXT:    [[I1:%.*]] = extractvalue { i32, i32 } [[AGG_00]], 1
 ; CHECK-NEXT:    br label [[BB0_MERGE:%.*]]
 ; CHECK:       bb01:
+; CHECK-NEXT:    [[I2:%.*]] = extractvalue { i32, i32 } [[AGG_01:%.*]], 0
+; CHECK-NEXT:    [[I3:%.*]] = extractvalue { i32, i32 } [[AGG_01]], 1
 ; CHECK-NEXT:    br label [[BB0_MERGE]]
 ; CHECK:       bb0.merge:
-; CHECK-NEXT:    [[AGG_00_PN:%.*]] = phi { i32, i32 } [ [[AGG_00:%.*]], [[BB00]] ], [ [[AGG_01:%.*]], [[BB01]] ]
-; CHECK-NEXT:    [[AGG_00_PN1:%.*]] = phi { i32, i32 } [ [[AGG_00]], [[BB00]] ], [ [[AGG_01]], [[BB01]] ]
+; CHECK-NEXT:    [[I4:%.*]] = phi i32 [ [[I0]], [[BB00]] ], [ [[I2]], [[BB01]] ]
+; CHECK-NEXT:    [[I5:%.*]] = phi i32 [ [[I1]], [[BB00]] ], [ [[I3]], [[BB01]] ]
 ; CHECK-NEXT:    br label [[END:%.*]]
 ; CHECK:       bb10:
+; CHECK-NEXT:    [[I6:%.*]] = extractvalue { i32, i32 } [[AGG_10:%.*]], 0
+; CHECK-NEXT:    [[I7:%.*]] = extractvalue { i32, i32 } [[AGG_10]], 1
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[AGG_00_PN_PN:%.*]] = phi { i32, i32 } [ [[AGG_00_PN]], [[BB0_MERGE]] ], [ [[AGG_10:%.*]], [[BB10]] ]
-; CHECK-NEXT:    [[AGG_00_PN1_PN:%.*]] = phi { i32, i32 } [ [[AGG_00_PN1]], [[BB0_MERGE]] ], [ [[AGG_10]], [[BB10]] ]
-; CHECK-NEXT:    [[I9:%.*]] = extractvalue { i32, i32 } [[AGG_00_PN1_PN]], 1
-; CHECK-NEXT:    [[I8:%.*]] = extractvalue { i32, i32 } [[AGG_00_PN_PN]], 0
+; CHECK-NEXT:    [[I8:%.*]] = phi i32 [ [[I4]], [[BB0_MERGE]] ], [ [[I6]], [[BB10]] ]
+; CHECK-NEXT:    [[I9:%.*]] = phi i32 [ [[I5]], [[BB0_MERGE]] ], [ [[I7]], [[BB10]] ]
 ; CHECK-NEXT:    call void @baz()
 ; CHECK-NEXT:    [[I10:%.*]] = insertvalue { i32, i32 } undef, i32 [[I8]], 0
 ; CHECK-NEXT:    [[I11:%.*]] = insertvalue { i32, i32 } [[I10]], i32 [[I9]], 1
@@ -277,17 +278,12 @@ define { i32, i32 } @test5({ i32, i32 } %agg_left, { i32, i32 } %agg_right, i1 %
 ; CHECK-NEXT:    call void @bar()
 ; CHECK-NEXT:    br label [[MIDDLE]]
 ; CHECK:       middle:
-; CHECK-NEXT:    [[AGG_LEFT_PN:%.*]] = phi { i32, i32 } [ [[AGG_LEFT:%.*]], [[LEFT]] ], [ [[AGG_RIGHT:%.*]], [[RIGHT]] ], [ [[I8:%.*]], [[MIDDLE]] ]
-; CHECK-NEXT:    [[AGG_LEFT_PN1:%.*]] = phi { i32, i32 } [ [[AGG_LEFT]], [[LEFT]] ], [ [[AGG_RIGHT]], [[RIGHT]] ], [ [[I8]], [[MIDDLE]] ]
-; CHECK-NEXT:    [[I6:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT_PN1]], 1
-; CHECK-NEXT:    [[I5:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT_PN]], 0
+; CHECK-NEXT:    [[I8_MERGED:%.*]] = phi { i32, i32 } [ [[I8_MERGED]], [[MIDDLE]] ], [ [[AGG_RIGHT:%.*]], [[RIGHT]] ], [ [[AGG_LEFT:%.*]], [[LEFT]] ]
 ; CHECK-NEXT:    call void @baz()
-; CHECK-NEXT:    [[I7:%.*]] = insertvalue { i32, i32 } undef, i32 [[I5]], 0
-; CHECK-NEXT:    [[I8]] = insertvalue { i32, i32 } [[I7]], i32 [[I6]], 1
 ; CHECK-NEXT:    [[C1:%.*]] = call i1 @geni1()
 ; CHECK-NEXT:    br i1 [[C1]], label [[END:%.*]], label [[MIDDLE]]
 ; CHECK:       end:
-; CHECK-NEXT:    ret { i32, i32 } [[I8]]
+; CHECK-NEXT:    ret { i32, i32 } [[I8_MERGED]]
 ;
 entry:
   br i1 %c0, label %left, label %right
@@ -331,20 +327,15 @@ define { i32, i32 } @test6({ i32, i32 } %agg_left, { i32, i32 } %agg_right, i1 %
 ; CHECK-NEXT:    call void @bar()
 ; CHECK-NEXT:    br label [[MERGE]]
 ; CHECK:       merge:
-; CHECK-NEXT:    [[AGG_LEFT_PN:%.*]] = phi { i32, i32 } [ [[AGG_LEFT:%.*]], [[LEFT]] ], [ [[AGG_RIGHT:%.*]], [[RIGHT]] ]
-; CHECK-NEXT:    [[AGG_LEFT_PN1:%.*]] = phi { i32, i32 } [ [[AGG_LEFT]], [[LEFT]] ], [ [[AGG_RIGHT]], [[RIGHT]] ]
+; CHECK-NEXT:    [[I8_MERGED:%.*]] = phi { i32, i32 } [ [[AGG_RIGHT:%.*]], [[RIGHT]] ], [ [[AGG_LEFT:%.*]], [[LEFT]] ]
 ; CHECK-NEXT:    call void @baz()
 ; CHECK-NEXT:    br i1 [[C1:%.*]], label [[END:%.*]], label [[PASSTHROUGH:%.*]]
 ; CHECK:       passthrough:
 ; CHECK-NEXT:    call void @qux()
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[I6:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT_PN1]], 1
-; CHECK-NEXT:    [[I5:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT_PN]], 0
 ; CHECK-NEXT:    call void @quux()
-; CHECK-NEXT:    [[I7:%.*]] = insertvalue { i32, i32 } undef, i32 [[I5]], 0
-; CHECK-NEXT:    [[I8:%.*]] = insertvalue { i32, i32 } [[I7]], i32 [[I6]], 1
-; CHECK-NEXT:    ret { i32, i32 } [[I8]]
+; CHECK-NEXT:    ret { i32, i32 } [[I8_MERGED]]
 ;
 entry:
   br i1 %c0, label %left, label %right
@@ -499,14 +490,9 @@ define { i32, i32 } @test9({ i32, i32 } %agg_left, { i32, i32 } %agg_right, i1 %
 ; CHECK-NEXT:    call void @bar()
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[AGG_LEFT_PN:%.*]] = phi { i32, i32 } [ [[AGG_LEFT:%.*]], [[LEFT]] ], [ [[AGG_RIGHT:%.*]], [[RIGHT]] ]
-; CHECK-NEXT:    [[AGG_LEFT_PN1:%.*]] = phi { i32, i32 } [ [[AGG_LEFT]], [[LEFT]] ], [ [[AGG_RIGHT]], [[RIGHT]] ]
-; CHECK-NEXT:    [[I7:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT_PN1]], 1
-; CHECK-NEXT:    [[I0_PN:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT_PN]], 0
-; CHECK-NEXT:    [[I6:%.*]] = insertvalue { i32, i32 } undef, i32 [[I0_PN]], 0
+; CHECK-NEXT:    [[I8_MERGED:%.*]] = phi { i32, i32 } [ [[AGG_RIGHT:%.*]], [[RIGHT]] ], [ [[AGG_LEFT:%.*]], [[LEFT]] ]
 ; CHECK-NEXT:    call void @baz()
-; CHECK-NEXT:    [[I8:%.*]] = insertvalue { i32, i32 } [[I6]], i32 [[I7]], 1
-; CHECK-NEXT:    ret { i32, i32 } [[I8]]
+; CHECK-NEXT:    ret { i32, i32 } [[I8_MERGED]]
 ;
 entry:
   br i1 %c, label %left, label %right

diff  --git a/llvm/test/Transforms/InstCombine/phi-of-extractvalues.ll b/llvm/test/Transforms/InstCombine/phi-of-extractvalues.ll
index d00915008b97..72abd9d64adb 100644
--- a/llvm/test/Transforms/InstCombine/phi-of-extractvalues.ll
+++ b/llvm/test/Transforms/InstCombine/phi-of-extractvalues.ll
@@ -10,12 +10,13 @@ define i32 @test0({ i32, i32 } %agg_left, { i32, i32 } %agg_right, i1 %c) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[LEFT:%.*]], label [[RIGHT:%.*]]
 ; CHECK:       left:
+; CHECK-NEXT:    [[I0:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT:%.*]], 0
 ; CHECK-NEXT:    br label [[END:%.*]]
 ; CHECK:       right:
+; CHECK-NEXT:    [[I1:%.*]] = extractvalue { i32, i32 } [[AGG_RIGHT:%.*]], 0
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[AGG_LEFT_PN:%.*]] = phi { i32, i32 } [ [[AGG_LEFT:%.*]], [[LEFT]] ], [ [[AGG_RIGHT:%.*]], [[RIGHT]] ]
-; CHECK-NEXT:    [[R:%.*]] = extractvalue { i32, i32 } [[AGG_LEFT_PN]], 0
+; CHECK-NEXT:    [[R:%.*]] = phi i32 [ [[I0]], [[LEFT]] ], [ [[I1]], [[RIGHT]] ]
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
 entry:
@@ -168,12 +169,13 @@ define i32 @test5({{ i32, i32 }, { i32, i32 }} %agg_left, {{ i32, i32 }, { i32,
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[LEFT:%.*]], label [[RIGHT:%.*]]
 ; CHECK:       left:
+; CHECK-NEXT:    [[I0:%.*]] = extractvalue { { i32, i32 }, { i32, i32 } } [[AGG_LEFT:%.*]], 0, 0
 ; CHECK-NEXT:    br label [[END:%.*]]
 ; CHECK:       right:
+; CHECK-NEXT:    [[I1:%.*]] = extractvalue { { i32, i32 }, { i32, i32 } } [[AGG_RIGHT:%.*]], 0, 0
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[AGG_LEFT_PN:%.*]] = phi { { i32, i32 }, { i32, i32 } } [ [[AGG_LEFT:%.*]], [[LEFT]] ], [ [[AGG_RIGHT:%.*]], [[RIGHT]] ]
-; CHECK-NEXT:    [[R:%.*]] = extractvalue { { i32, i32 }, { i32, i32 } } [[AGG_LEFT_PN]], 0, 0
+; CHECK-NEXT:    [[R:%.*]] = phi i32 [ [[I0]], [[LEFT]] ], [ [[I1]], [[RIGHT]] ]
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
 entry:


        


More information about the llvm-commits mailing list