Identified regression caused by *llvm:2eb554a9feafff5188d8b924908205c87d7f2fee*:
commit 2eb554a9feafff5188d8b924908205c87d7f2fee
Author: Roman Lebedev <lebedev...@gmail.com>

    Revert "Reland [SimplifyCFG] performBranchToCommonDestFolding(): form 
block-closed SSA form before cloning instructions (PR51125)"

Results regressed to (for first_bad == 2eb554a9feafff5188d8b924908205c87d7f2fee)
# reset_artifacts:
-10
# build_abe binutils:
-9
# build_abe stage1 -- --set gcc_override_configure=--with-mode=arm --set 
gcc_override_configure=--disable-libsanitizer:
-8
# build_abe linux:
-7
# build_abe glibc:
-6
# build_abe stage2 -- --set gcc_override_configure=--with-mode=arm --set 
gcc_override_configure=--disable-libsanitizer:
-5
# build_llvm true:
-3
# true:
0
# benchmark -- -O3_marm 
artifacts/build-2eb554a9feafff5188d8b924908205c87d7f2fee/results_id:
1
# 483.xalancbmk,libc.so.6                                       regressed by 
81400

from (for last_good == 7142eb17fb3419a76c9ac4afce0df986ff08d61c)
# reset_artifacts:
-10
# build_abe binutils:
-9
# build_abe stage1 -- --set gcc_override_configure=--with-mode=arm --set 
gcc_override_configure=--disable-libsanitizer:
-8
# build_abe linux:
-7
# build_abe glibc:
-6
# build_abe stage2 -- --set gcc_override_configure=--with-mode=arm --set 
gcc_override_configure=--disable-libsanitizer:
-5
# build_llvm true:
-3
# true:
0
# benchmark -- -O3_marm 
artifacts/build-7142eb17fb3419a76c9ac4afce0df986ff08d61c/results_id:
1

This commit has regressed these CI configurations:
 - tcwg_bmk_llvm_tk1/llvm-master-arm-spec2k6-O3

Artifacts of last_good build: 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tk1-llvm-master-arm-spec2k6-O3/17/artifact/artifacts/build-7142eb17fb3419a76c9ac4afce0df986ff08d61c/
Artifacts of first_bad build: 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tk1-llvm-master-arm-spec2k6-O3/17/artifact/artifacts/build-2eb554a9feafff5188d8b924908205c87d7f2fee/
Even more details: 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tk1-llvm-master-arm-spec2k6-O3/17/artifact/artifacts/

Reproduce builds:
<cut>
mkdir investigate-llvm-2eb554a9feafff5188d8b924908205c87d7f2fee
cd investigate-llvm-2eb554a9feafff5188d8b924908205c87d7f2fee

# Fetch scripts
git clone https://git.linaro.org/toolchain/jenkins-scripts

# Fetch manifests and test.sh script
mkdir -p artifacts/manifests
curl -o artifacts/manifests/build-baseline.sh 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tk1-llvm-master-arm-spec2k6-O3/17/artifact/artifacts/manifests/build-baseline.sh
 --fail
curl -o artifacts/manifests/build-parameters.sh 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tk1-llvm-master-arm-spec2k6-O3/17/artifact/artifacts/manifests/build-parameters.sh
 --fail
curl -o artifacts/test.sh 
https://ci.linaro.org/job/tcwg_bmk_ci_llvm-bisect-tcwg_bmk_tk1-llvm-master-arm-spec2k6-O3/17/artifact/artifacts/test.sh
 --fail
chmod +x artifacts/test.sh

# Reproduce the baseline build (build all pre-requisites)
./jenkins-scripts/tcwg_bmk-build.sh @@ artifacts/manifests/build-baseline.sh

# Save baseline build state (which is then restored in artifacts/test.sh)
mkdir -p ./bisect
rsync -a --del --delete-excluded --exclude /bisect/ --exclude /artifacts/ 
--exclude /llvm/ ./ ./bisect/baseline/

cd llvm

# Reproduce first_bad build
git checkout --detach 2eb554a9feafff5188d8b924908205c87d7f2fee
../artifacts/test.sh

# Reproduce last_good build
git checkout --detach 7142eb17fb3419a76c9ac4afce0df986ff08d61c
../artifacts/test.sh

cd ..
</cut>

Full commit (up to 1000 lines):
<cut>
commit 2eb554a9feafff5188d8b924908205c87d7f2fee
Author: Roman Lebedev <lebedev...@gmail.com>
Date:   Mon Aug 16 10:53:15 2021 +0300

    Revert "Reland [SimplifyCFG] performBranchToCommonDestFolding(): form 
block-closed SSA form before cloning instructions (PR51125)"
    
    This is still wrong, as failing bots suggest.
    
    This reverts commit 3d9beefc7d713ad8462d92427ccd17b9532ce904.
---
 llvm/lib/Transforms/Utils/SimplifyCFG.cpp          | 75 +++-------------------
 .../SimplifyCFG/fold-branch-to-common-dest.ll      | 18 +++---
 2 files changed, 18 insertions(+), 75 deletions(-)

diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp 
b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 68a0388398fc..847fdd760d2f 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1095,24 +1095,17 @@ static void 
CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
 
     // Update (liveout) uses of bonus instructions,
     // now that the bonus instruction has been cloned into predecessor.
-    // Note that we expect to be in a block-closed SSA form for this to work!
+    SSAUpdater SSAUpdate;
+    SSAUpdate.Initialize(BonusInst.getType(),
+                         (NewBonusInst->getName() + ".merge").str());
+    SSAUpdate.AddAvailableValue(BB, &BonusInst);
+    SSAUpdate.AddAvailableValue(PredBlock, NewBonusInst);
     for (Use &U : make_early_inc_range(BonusInst.uses())) {
       auto *UI = cast<Instruction>(U.getUser());
-      auto *PN = dyn_cast<PHINode>(UI);
-      if (!PN) {
-        assert(UI->getParent() == BB && BonusInst.comesBefore(UI) &&
-               "If the user is not a PHI node, then it should be in the same "
-               "block as, and come after, the original bonus instruction.");
-        continue; // Keep using the original bonus instruction.
-      }
-      // Is this the block-closed SSA form PHI node?
-      if (PN->getIncomingBlock(U) == BB)
-        continue; // Great, keep using the original bonus instruction.
-      // The only other alternative is an "use" when coming from
-      // the predecessor block - here we should refer to the cloned bonus 
instr.
-      assert(PN->getIncomingBlock(U) == PredBlock &&
-             "Not in block-closed SSA form?");
-      U.set(NewBonusInst);
+      if (UI->getParent() != PredBlock)
+        SSAUpdate.RewriteUseAfterInsertions(U);
+      else // Use is in the same block as, and comes before, NewBonusInst.
+        SSAUpdate.RewriteUse(U);
     }
   }
 }
@@ -3039,56 +3032,6 @@ static bool performBranchToCommonDestFolding(BranchInst 
*BI, BranchInst *PBI,
 
   LLVM_DEBUG(dbgs() << "FOLDING BRANCH TO COMMON DEST:\n" << *PBI << *BB);
 
-  // We want to duplicate all the bonus instructions in this block,
-  // and rewrite their uses, but in some cases with self-loops,
-  // the naive use rewrite approach won't work (will result in 
miscompilations).
-  // To avoid this problem, let's form block-closed SSA form.
-  for (Instruction &BonusInst :
-       reverse(iterator_range<BasicBlock::iterator>(*BB))) {
-    auto IsBCSSAUse = [BB, &BonusInst](Use &U) {
-      auto *UI = cast<Instruction>(U.getUser());
-      if (auto *PN = dyn_cast<PHINode>(UI))
-        return PN->getIncomingBlock(U) == BB;
-      return UI->getParent() == BB && BonusInst.comesBefore(UI);
-    };
-
-    // Does this instruction require rewriting of uses?
-    if (all_of(BonusInst.uses(), IsBCSSAUse))
-      continue;
-
-    SSAUpdater SSAUpdate;
-    Type *Ty = BonusInst.getType();
-    SmallVector<PHINode *, 8> BCSSAPHIs;
-    SSAUpdate.Initialize(Ty, BonusInst.getName());
-
-    // Into each successor block of BB, insert a PHI node, that receives
-    // the BonusInst when coming from it's basic block, or poison otherwise.
-    for (BasicBlock *Succ : successors(BB)) {
-      // The block may have the same successor multiple times. Do it only once.
-      if (SSAUpdate.HasValueForBlock(Succ))
-        continue;
-      BCSSAPHIs.emplace_back(PHINode::Create(
-          Ty, 0, BonusInst.getName() + ".bcssa", &Succ->front()));
-      PHINode *PN = BCSSAPHIs.back();
-      for (BasicBlock *PredOfSucc : predecessors(Succ))
-        PN->addIncoming(PredOfSucc == BB ? (Value *)&BonusInst
-                                         : PoisonValue::get(Ty),
-                        PredOfSucc);
-      SSAUpdate.AddAvailableValue(Succ, PN);
-    }
-
-    // And rewrite all uses that break block-closed SSA form.
-    for (Use &U : make_early_inc_range(BonusInst.uses()))
-      if (!IsBCSSAUse(U))
-        SSAUpdate.RewriteUseAfterInsertions(U);
-
-    // We might not have ended up needing PHI's in all of the succ blocks,
-    // drop the ones that are certainly unused, but don't bother otherwise.
-    for (PHINode *PN : BCSSAPHIs)
-      if (PN->use_empty())
-        PN->eraseFromParent();
-  }
-
   IRBuilder<> Builder(PBI);
   // The builder is used to create instructions to eliminate the branch in BB.
   // If BB's terminator has !annotation metadata, add it to the new
diff --git a/llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll 
b/llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll
index d948b61d65a0..2ff041826077 100644
--- a/llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll
+++ b/llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll
@@ -834,7 +834,7 @@ define void @pr48450() {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
 ; CHECK:       for.body:
-; CHECK-NEXT:    [[COUNTDOWN:%.*]] = phi i8 [ 8, [[ENTRY:%.*]] ], [ 
[[DEC_BCSSA1:%.*]], [[FOR_BODYTHREAD_PRE_SPLIT:%.*]] ]
+; CHECK-NEXT:    [[COUNTDOWN:%.*]] = phi i8 [ 8, [[ENTRY:%.*]] ], [ 
[[DEC_MERGE:%.*]], [[FOR_BODYTHREAD_PRE_SPLIT:%.*]] ]
 ; CHECK-NEXT:    [[C:%.*]] = call i1 @gen1()
 ; CHECK-NEXT:    br i1 [[C]], label [[FOR_INC:%.*]], label [[IF_THEN:%.*]]
 ; CHECK:       for.inc:
@@ -849,7 +849,7 @@ define void @pr48450() {
 ; CHECK-NEXT:    [[OR_COND:%.*]] = select i1 [[C2_NOT]], i1 true, i1 
[[CMP_NOT]]
 ; CHECK-NEXT:    br i1 [[OR_COND]], label [[IF_END_LOOPEXIT]], label 
[[FOR_BODYTHREAD_PRE_SPLIT]]
 ; CHECK:       for.bodythread-pre-split:
-; CHECK-NEXT:    [[DEC_BCSSA1]] = phi i8 [ [[DEC_OLD]], [[FOR_INC]] ], [ 
[[DEC]], [[IF_THEN]] ]
+; CHECK-NEXT:    [[DEC_MERGE]] = phi i8 [ [[DEC]], [[IF_THEN]] ], [ 
[[DEC_OLD]], [[FOR_INC]] ]
 ; CHECK-NEXT:    call void @sideeffect0()
 ; CHECK-NEXT:    br label [[FOR_BODY]]
 ; CHECK:       if.end.loopexit:
@@ -885,7 +885,7 @@ define void @pr48450_2(i1 %enable_loopback) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br label [[FOR_BODY:%.*]]
 ; CHECK:       for.body:
-; CHECK-NEXT:    [[COUNTDOWN:%.*]] = phi i8 [ 8, [[ENTRY:%.*]] ], [ 
[[DEC_BCSSA1:%.*]], [[FOR_BODYTHREAD_PRE_SPLIT:%.*]] ]
+; CHECK-NEXT:    [[COUNTDOWN:%.*]] = phi i8 [ 8, [[ENTRY:%.*]] ], [ 
[[DEC_MERGE:%.*]], [[FOR_BODYTHREAD_PRE_SPLIT:%.*]] ]
 ; CHECK-NEXT:    [[C:%.*]] = call i1 @gen1()
 ; CHECK-NEXT:    br i1 [[C]], label [[FOR_INC:%.*]], label [[IF_THEN:%.*]]
 ; CHECK:       for.inc:
@@ -900,7 +900,7 @@ define void @pr48450_2(i1 %enable_loopback) {
 ; CHECK-NEXT:    [[OR_COND:%.*]] = select i1 [[C2_NOT]], i1 true, i1 
[[CMP_NOT]]
 ; CHECK-NEXT:    br i1 [[OR_COND]], label [[IF_END_LOOPEXIT]], label 
[[FOR_BODYTHREAD_PRE_SPLIT]]
 ; CHECK:       for.bodythread-pre-split:
-; CHECK-NEXT:    [[DEC_BCSSA1]] = phi i8 [ poison, 
[[FOR_BODYTHREAD_PRE_SPLIT_LOOPBACK:%.*]] ], [ [[DEC_OLD]], [[FOR_INC]] ], [ 
[[DEC]], [[IF_THEN]] ]
+; CHECK-NEXT:    [[DEC_MERGE]] = phi i8 [ [[DEC_OLD]], [[FOR_INC]] ], [ 
[[DEC_MERGE]], [[FOR_BODYTHREAD_PRE_SPLIT_LOOPBACK:%.*]] ], [ [[DEC]], 
[[IF_THEN]] ]
 ; CHECK-NEXT:    [[SHOULD_LOOPBACK:%.*]] = phi i1 [ true, [[FOR_INC]] ], [ 
false, [[FOR_BODYTHREAD_PRE_SPLIT_LOOPBACK]] ], [ true, [[IF_THEN]] ]
 ; CHECK-NEXT:    [[DO_LOOPBACK:%.*]] = and i1 [[SHOULD_LOOPBACK]], 
[[ENABLE_LOOPBACK:%.*]]
 ; CHECK-NEXT:    call void @sideeffect0()
@@ -1005,8 +1005,8 @@ define void @pr49510() {
 ; CHECK-NEXT:    [[TOBOOL_OLD:%.*]] = icmp ne i16 [[DOTOLD]], 0
 ; CHECK-NEXT:    br i1 [[TOBOOL_OLD]], label [[LAND_RHS:%.*]], label 
[[FOR_END:%.*]]
 ; CHECK:       land.rhs:
-; CHECK-NEXT:    [[DOTBCSSA:%.*]] = phi i16 [ [[DOTOLD]], [[ENTRY:%.*]] ], [ 
[[TMP0:%.*]], [[LAND_RHS]] ]
-; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i16 [[DOTBCSSA]], 0
+; CHECK-NEXT:    [[DOTMERGE:%.*]] = phi i16 [ [[TMP0:%.*]], [[LAND_RHS]] ], [ 
[[DOTOLD]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i16 [[DOTMERGE]], 0
 ; CHECK-NEXT:    [[TMP0]] = load i16, i16* @global_pr49510, align 1
 ; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp ne i16 [[TMP0]], 0
 ; CHECK-NEXT:    [[OR_COND:%.*]] = select i1 [[CMP]], i1 [[TOBOOL]], i1 false
@@ -1043,15 +1043,15 @@ define i32 @pr51125() {
 ; CHECK-NEXT:    [[ISZERO_OLD:%.*]] = icmp eq i32 [[LD_OLD]], 0
 ; CHECK-NEXT:    br i1 [[ISZERO_OLD]], label [[EXIT:%.*]], label [[L2:%.*]]
 ; CHECK:       L2:
-; CHECK-NEXT:    [[LD_BCSSA1:%.*]] = phi i32 [ [[LD_OLD]], [[ENTRY:%.*]] ], [ 
[[LD:%.*]], [[L2]] ]
+; CHECK-NEXT:    [[LD_MERGE:%.*]] = phi i32 [ [[LD:%.*]], [[L2]] ], [ 
[[LD_OLD]], [[ENTRY:%.*]] ]
 ; CHECK-NEXT:    store i32 -1, i32* @global_pr51125, align 4
-; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32 [[LD_BCSSA1]], -1
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32 [[LD_MERGE]], -1
 ; CHECK-NEXT:    [[LD]] = load i32, i32* @global_pr51125, align 4
 ; CHECK-NEXT:    [[ISZERO:%.*]] = icmp eq i32 [[LD]], 0
 ; CHECK-NEXT:    [[OR_COND:%.*]] = select i1 [[CMP]], i1 true, i1 [[ISZERO]]
 ; CHECK-NEXT:    br i1 [[OR_COND]], label [[EXIT]], label [[L2]]
 ; CHECK:       exit:
-; CHECK-NEXT:    [[R:%.*]] = phi i32 [ [[LD_BCSSA1]], [[L2]] ], [ [[LD_OLD]], 
[[ENTRY]] ]
+; CHECK-NEXT:    [[R:%.*]] = phi i32 [ [[LD]], [[L2]] ], [ [[LD_OLD]], 
[[ENTRY]] ]
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
 entry:
</cut>
_______________________________________________
linaro-toolchain mailing list
linaro-toolchain@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/linaro-toolchain

Reply via email to