Make FoldBranchToCommonDest poison-safe by default

[aqjune]

Summary

This is a small patch to make FoldBranchToCommonDest poison-safe by default.
After fc3f0c9c, only two syntactic changes are needed to fix unit tests.
This does not cause any assembly difference in testsuite as well (-O3, X86-64 Manjaro).

[nikic]

llvm/test/Transforms/IRCE/bad_expander.ll

101

This looks a bit suspect to me. Shouldn't the impliesPoison turn this into an and, because true cannot be poison? Or does this only get folded to true lateron?

[aqjune]

llvm/test/Transforms/IRCE/bad_expander.ll

101

FoldBranchToCommonDest merges %maybe_exit and %rc, so select is created at the moment. %rc is folded to true later.

[nikic]

LGTM

llvm/include/llvm/Transforms/Utils/Local.h

 /// This function is used to flatten a CFG. For example, it uses parallel-and
 /// and parallel-or mode to collapse if-conditions and merge if-regions with
 /// identical statements.
 bool FlattenCFG(BasicBlock *BB, AAResults *AA = nullptr);
 
 /// If this basic block is ONLY a setcc and a branch, and if a predecessor
 /// branches to us and one of our successors, fold the setcc into the
 /// predecessor and use logical operations to pick the right destination.
-/// If PoisonSafe is true, use select i1 rather than and/or i1 to successfully
-/// block unexpected propagation of poison when merging the branches. This is
-/// set to false by default when used by LoopSimplify for performance, but this
-/// should be turned on by default.
 bool FoldBranchToCommonDest(BranchInst *BI, llvm::DomTreeUpdater *DTU = nullptr,
                             MemorySSAUpdater *MSSAU = nullptr,
                             const TargetTransformInfo *TTI = nullptr,
-                            unsigned BonusInstThreshold = 1,
-                            bool PoisonSafe = false);
+                            unsigned BonusInstThreshold = 1);
 
 /// This function takes a virtual register computed by an Instruction and
 /// replaces it with a slot in the stack frame, allocated via alloca.
 /// This allows the CFG to be changed around without fear of invalidating the
 /// SSA information for the value. It returns the pointer to the alloca inserted
 /// to create a stack slot for X.
 AllocaInst *DemoteRegToStack(Instruction &X,
                              bool VolatileLoads = false,

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

if (PBITrueProb.isUnknown() || PBITrueProb.getCompl() < Likely)
       return {{Instruction::Or, true}};
   }
   return None;
 }
 
 static bool performBranchToCommonDestFolding(BranchInst *BI, BranchInst *PBI,
                                              DomTreeUpdater *DTU,
                                              MemorySSAUpdater *MSSAU,
-                                             bool PoisonSafe,
                                              const TargetTransformInfo *TTI) {
   BasicBlock *BB = BI->getParent();
   BasicBlock *PredBlock = PBI->getParent();
 
   // Determine if the two branches share a common destination.
   Instruction::BinaryOps Opc;
   bool InvertPredCond;
   std::tie(Opc, InvertPredCond) =
static bool performBranchToCommonDestFolding(BranchInst *BI, BranchInst *PBI,
 
   ValueToValueMapTy VMap; // maps original values to cloned values
   CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(BB, PredBlock, VMap);
 
   // Now that the Cond was cloned into the predecessor basic block,
   // or/and the two conditions together.
   Value *NewCond = nullptr;
   Value *BICond = VMap[BI->getCondition()];
-  bool UseBinOp = !PoisonSafe || impliesPoison(BICond, PBI->getCondition());
 
-  if (UseBinOp)
+  if (impliesPoison(BICond, PBI->getCondition()))
     NewCond = Builder.CreateBinOp(Opc, PBI->getCondition(), BICond, "or.cond");
   else
     NewCond =
         Opc == Instruction::And
             ? Builder.CreateLogicalAnd(PBI->getCondition(), BICond, "or.cond")
             : Builder.CreateLogicalOr(PBI->getCondition(), BICond, "or.cond");
   PBI->setCondition(NewCond);
 

 }
 
 /// If this basic block is simple enough, and if a predecessor branches to us
 /// and one of our successors, fold the block into the predecessor and use
 /// logical operations to pick the right destination.
 bool llvm::FoldBranchToCommonDest(BranchInst *BI, DomTreeUpdater *DTU,
                                   MemorySSAUpdater *MSSAU,
                                   const TargetTransformInfo *TTI,
-                                  unsigned BonusInstThreshold,
-                                  bool PoisonSafe) {
+                                  unsigned BonusInstThreshold) {
   // If this block ends with an unconditional branch,
   // let SpeculativelyExecuteBB() deal with it.
   if (!BI->isConditional())
     return false;
 
   BasicBlock *BB = BI->getParent();
 
   bool Changed = false;
for (Instruction &I : *BB) {
     // Early exits once we reach the limit.
     if (NumBonusInsts > BonusInstThreshold)
       return Changed;
   }
 
   // Ok, we have the budget. Perform the transformation.
   for (BasicBlock *PredBlock : Preds) {
     auto *PBI = cast<BranchInst>(PredBlock->getTerminator());
-    return performBranchToCommonDestFolding(BI, PBI, DTU, MSSAU, PoisonSafe,
-                                            TTI);
+    return performBranchToCommonDestFolding(BI, PBI, DTU, MSSAU, TTI);
   }
   return Changed;
 }
 
 // If there is only one store in BB1 and BB2, return it, otherwise return
 // nullptr.
 static StoreInst *findUniqueStoreInBlocks(BasicBlock *BB1, BasicBlock *BB2) {
   StoreInst *S = nullptr;
if (I->isTerminator() && TryToMergeLandingPad(LPad, BI, BB, DTU))
       return true;
   }
 
   // If this basic block is ONLY a compare and a branch, and if a predecessor
   // branches to us and our successor, fold the comparison into the
   // predecessor and use logical operations to update the incoming value
   // for PHI nodes in common successor.
   if (FoldBranchToCommonDest(BI, DTU, /*MSSAU=*/nullptr, &TTI,
-                             Options.BonusInstThreshold, true))
+                             Options.BonusInstThreshold))
     return requestResimplify();
   return false;
 }
 
 static BasicBlock *allPredecessorsComeFromSameSource(BasicBlock *BB) {
   BasicBlock *PredPred = nullptr;
   for (auto *P : predecessors(BB)) {
     BasicBlock *PPred = P->getSinglePredecessor();
if (Imp) {
     RecursivelyDeleteTriviallyDeadInstructions(OldCond);
     return requestResimplify();
   }
 
   // If this basic block is ONLY a compare and a branch, and if a predecessor
   // branches to us and one of our successors, fold the comparison into the
   // predecessor and use logical operations to pick the right destination.
   if (FoldBranchToCommonDest(BI, DTU, /*MSSAU=*/nullptr, &TTI,
-                             Options.BonusInstThreshold, true))
+                             Options.BonusInstThreshold))
     return requestResimplify();
 
   // We have a conditional branch to two blocks that are only reachable
   // from BI.  We know that the condbr dominates the two blocks, so see if
   // there is any identical code in the "then" and "else" blocks.  If so, we
   // can hoist it up to the branching block.
   if (BI->getSuccessor(0)->getSinglePredecessor()) {
     if (BI->getSuccessor(1)->getSinglePredecessor()) {

llvm/test/Transforms/IRCE/bad_expander.ll

 ; CHECK-NEXT:    %exit.mainloop.at = call i64 @llvm.smax.i64(i64 [[DIV_MINUS_1]], i64 0)
 ; CHECK-NEXT:    [[COMP2:%[^ ]+]] = icmp slt i64 0, %exit.mainloop.at
 ; CHECK-NEXT:    br i1 [[COMP2]], label %loop.preheader, label %main.pseudo.exit
 ; CHECK-NOT:     preloop
 ; CHECK:       loop:
 ; CHECK-NEXT:    %iv = phi i64 [ %iv.next, %guarded ], [ 0, %loop.preheader ]
 ; CHECK-NEXT:    %iv.next = add nuw nsw i64 %iv, 1
 ; CHECK-NEXT:    %rc = icmp slt i64 %iv.next, %div_result
-; CHECK-NEXT:    %or.cond = and i1 %maybe_exit, true
+; CHECK-NEXT:    %or.cond = select i1 %maybe_exit, i1 true, i1 false
 ; CHECK-NEXT:    br i1 %or.cond, label %guarded, label %exit.loopexit1
 ; CHECK:       guarded:
 ; CHECK-NEXT:    %gep = getelementptr i64, i64* %p1, i64 %iv.next
 ; CHECK-NEXT:    %loaded = load i64, i64* %gep, align 4
 ; CHECK-NEXT:    %tmp7 = icmp slt i64 %iv.next, 1000
 ; CHECK-NEXT:    [[EXIT_MAIN_LOOP:%[^ ]+]] = icmp slt i64 %iv.next, %exit.mainloop.at
 ; CHECK-NEXT:    br i1 [[EXIT_MAIN_LOOP]], label %loop, label %main.exit.selector
 ; CHECK:       postloop

llvm/test/Transforms/LoopSimplify/pr26682.ll

 ; RUN: opt < %s -lcssa -loop-simplify -indvars -S | FileCheck %s
 target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-unknown"
 
 @a = external global i32, align 4
 
 ; Check that loop-simplify merges two loop exits, but preserves LCSSA form.
 ; CHECK-LABEL: @foo
 ; CHECK: for:
-; CHECK: %or.cond = and i1 %cmp1, %cmp2
+; CHECK: %or.cond = select i1 %cmp1, i1 %cmp2, i1 false
 ; CHECK-NOT: for.cond:
 ; CHECK: for.end:
 ; CHECK: %a.lcssa = phi i32 [ %a, %for ]
 define i32 @foo(i32 %x) {
 entry:
   br label %for
 
 for: