[llvm] 571c713 - [SimplifyCFG] Handle trapping aggregates (PR49839)

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2022-06-13T14:56:49+02:00
New Revision: 571c7131444d6e4e92f002e4f136d26087f36810

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

LOG: [SimplifyCFG] Handle trapping aggregates (PR49839)

Handle the fact that not only constant expressions, but also
constant aggregates containing expressions can trap.

This still doesn't fix the original C reproducer, probably due to
more issues remaining in other passes.

Added: 
    

Modified: 
    llvm/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/test/Transforms/SimplifyCFG/ConditionalTrappingConstantExpr.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 067d432cd19b7..76cadfe687bed 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -382,6 +382,13 @@ static InstructionCost computeSpeculationCost(const User *I,
   return TTI.getUserCost(I, TargetTransformInfo::TCK_SizeAndLatency);
 }
 
+/// Check whether this is a potentially trapping constant.
+static bool canTrap(const Value *V) {
+  if (auto *C = dyn_cast<Constant>(V))
+    return C->canTrap();
+  return false;
+}
+
 /// If we have a merge point of an "if condition" as accepted above,
 /// return true if the specified value dominates the block.  We
 /// don't handle the true generality of domination here, just a special case
@@ -416,10 +423,7 @@ static bool dominatesMergePoint(Value *V, BasicBlock *BB,
   if (!I) {
     // Non-instructions all dominate instructions, but not all constantexprs
     // can be executed unconditionally.
-    if (ConstantExpr *C = dyn_cast<ConstantExpr>(V))
-      if (C->canTrap())
-        return false;
-    return true;
+    return !canTrap(V);
   }
   BasicBlock *PBB = I->getParent();
 
@@ -2675,15 +2679,15 @@ static bool validateAndCostRequiredSelects(BasicBlock *BB, BasicBlock *ThenBB,
         passingValueIsAlwaysUndefined(ThenV, &PN))
       return false;
 
+    if (canTrap(OrigV) || canTrap(ThenV))
+      return false;
+
     HaveRewritablePHIs = true;
     ConstantExpr *OrigCE = dyn_cast<ConstantExpr>(OrigV);
     ConstantExpr *ThenCE = dyn_cast<ConstantExpr>(ThenV);
     if (!OrigCE && !ThenCE)
-      continue; // Known safe and cheap.
+      continue; // Known cheap (FIXME: Maybe not true for aggregates).
 
-    if ((ThenCE && !isSafeToSpeculativelyExecute(ThenCE)) ||
-        (OrigCE && !isSafeToSpeculativelyExecute(OrigCE)))
-      return false;
     InstructionCost OrigCost = OrigCE ? computeSpeculationCost(OrigCE, TTI) : 0;
     InstructionCost ThenCost = ThenCE ? computeSpeculationCost(ThenCE, TTI) : 0;
     InstructionCost MaxCost =
@@ -3591,12 +3595,10 @@ bool llvm::FoldBranchToCommonDest(BranchInst *BI, DomTreeUpdater *DTU,
 
   // Cond is known to be a compare or binary operator.  Check to make sure that
   // neither operand is a potentially-trapping constant expression.
-  if (ConstantExpr *CE = dyn_cast<ConstantExpr>(Cond->getOperand(0)))
-    if (CE->canTrap())
-      return false;
-  if (ConstantExpr *CE = dyn_cast<ConstantExpr>(Cond->getOperand(1)))
-    if (CE->canTrap())
-      return false;
+  if (canTrap(Cond->getOperand(0)))
+    return false;
+  if (canTrap(Cond->getOperand(1)))
+    return false;
 
   // Finally, don't infinitely unroll conditional loops.
   if (is_contained(successors(BB), BB))
@@ -4105,9 +4107,8 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
   if (tryWidenCondBranchToCondBranch(PBI, BI, DTU))
     return true;
 
-  if (auto *CE = dyn_cast<ConstantExpr>(BI->getCondition()))
-    if (CE->canTrap())
-      return false;
+  if (canTrap(BI->getCondition()))
+    return false;
 
   // If both branches are conditional and both contain stores to the same
   // address, remove the stores from the conditionals and create a conditional
@@ -4164,15 +4165,13 @@ static bool SimplifyCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
 
     PHINode *PN = cast<PHINode>(II);
     Value *BIV = PN->getIncomingValueForBlock(BB);
-    if (ConstantExpr *CE = dyn_cast<ConstantExpr>(BIV))
-      if (CE->canTrap())
-        return false;
+    if (canTrap(BIV))
+      return false;
 
     unsigned PBBIdx = PN->getBasicBlockIndex(PBI->getParent());
     Value *PBIV = PN->getIncomingValue(PBBIdx);
-    if (ConstantExpr *CE = dyn_cast<ConstantExpr>(PBIV))
-      if (CE->canTrap())
-        return false;
+    if (canTrap(PBIV))
+      return false;
   }
 
   // Finally, if everything is ok, fold the branches to logical ops.

diff  --git a/llvm/test/Transforms/SimplifyCFG/ConditionalTrappingConstantExpr.ll b/llvm/test/Transforms/SimplifyCFG/ConditionalTrappingConstantExpr.ll
index 6d827ef9ccc43..ce4eca16bfdf3 100644
--- a/llvm/test/Transforms/SimplifyCFG/ConditionalTrappingConstantExpr.ll
+++ b/llvm/test/Transforms/SimplifyCFG/ConditionalTrappingConstantExpr.ll
@@ -72,8 +72,12 @@ bb10:
 define <1 x i64> @trapping_const_agg(i1 %c) {
 ; CHECK-LABEL: @trapping_const_agg(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[SPEC_SELECT:%.*]] = select i1 [[C:%.*]], <1 x i64> <i64 srem (i64 1, i64 ptrtoint (i32* @g to i64))>, <1 x i64> zeroinitializer
-; CHECK-NEXT:    ret <1 x i64> [[SPEC_SELECT]]
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[IF:%.*]], label [[END:%.*]]
+; CHECK:       if:
+; CHECK-NEXT:    br label [[END]]
+; CHECK:       end:
+; CHECK-NEXT:    [[PHI:%.*]] = phi <1 x i64> [ zeroinitializer, [[ENTRY:%.*]] ], [ <i64 srem (i64 1, i64 ptrtoint (i32* @g to i64))>, [[IF]] ]
+; CHECK-NEXT:    ret <1 x i64> [[PHI]]
 ;
 entry:
   br i1 %c, label %if, label %end