[llvm] 8ba56f3 - Move widenable branch formation into makeGuardControlFlowExplicit helper

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2019-11-20T12:54:05-08:00
New Revision: 8ba56f322abf848cec78ff7f814f3ad84cd778be

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

LOG: Move widenable branch formation into makeGuardControlFlowExplicit helper

This is mostly NFC, but I removed the setting of the guard's calling convention onto the WC call.  Why?  Because it was untested, and was producing an ill defined output as the declaration's convention wasn't been changed leaving a mismatch which is UB.

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/GuardUtils.h
    llvm/lib/Transforms/Scalar/LowerGuardIntrinsic.cpp
    llvm/lib/Transforms/Scalar/MakeGuardsExplicit.cpp
    llvm/lib/Transforms/Utils/GuardUtils.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Utils/GuardUtils.h b/llvm/include/llvm/Transforms/Utils/GuardUtils.h
index 43cd267f59bc..7ab5d9ef4f23 100644
--- a/llvm/include/llvm/Transforms/Utils/GuardUtils.h
+++ b/llvm/include/llvm/Transforms/Utils/GuardUtils.h
@@ -23,8 +23,11 @@ class Value;
 /// by the condition of guard's first argument. The taken branch then goes to
 /// the block that contains  \p Guard's successors, and the non-taken branch
 /// goes to a newly-created deopt block that contains a sole call of the
-/// deoptimize function \p DeoptIntrinsic.
-void makeGuardControlFlowExplicit(Function *DeoptIntrinsic, CallInst *Guard);
+/// deoptimize function \p DeoptIntrinsic.  If 'UseWC' is set, preserve the
+/// widenable nature of the guard by lowering to equivelent form.  If not set,
+/// lower to a form without widenable semantics.
+void makeGuardControlFlowExplicit(Function *DeoptIntrinsic, CallInst *Guard,
+                                  bool UseWC);
 
 /// Given a branch we know is widenable (defined per Analysis/GuardUtils.h),
 /// widen it such that condition 'NewCond' is also known to hold on the taken

diff  --git a/llvm/lib/Transforms/Scalar/LowerGuardIntrinsic.cpp b/llvm/lib/Transforms/Scalar/LowerGuardIntrinsic.cpp
index f8aa2802cce7..45f5929e3b90 100644
--- a/llvm/lib/Transforms/Scalar/LowerGuardIntrinsic.cpp
+++ b/llvm/lib/Transforms/Scalar/LowerGuardIntrinsic.cpp
@@ -61,7 +61,7 @@ static bool lowerGuardIntrinsic(Function &F) {
   DeoptIntrinsic->setCallingConv(GuardDecl->getCallingConv());
 
   for (auto *CI : ToLower) {
-    makeGuardControlFlowExplicit(DeoptIntrinsic, CI);
+    makeGuardControlFlowExplicit(DeoptIntrinsic, CI, false);
     CI->eraseFromParent();
   }
 

diff  --git a/llvm/lib/Transforms/Scalar/MakeGuardsExplicit.cpp b/llvm/lib/Transforms/Scalar/MakeGuardsExplicit.cpp
index fd8a3643cfff..0e3fcf0eb775 100644
--- a/llvm/lib/Transforms/Scalar/MakeGuardsExplicit.cpp
+++ b/llvm/lib/Transforms/Scalar/MakeGuardsExplicit.cpp
@@ -58,22 +58,9 @@ struct MakeGuardsExplicitLegacyPass : public FunctionPass {
 static void turnToExplicitForm(CallInst *Guard, Function *DeoptIntrinsic) {
   // Replace the guard with an explicit branch (just like in GuardWidening).
   BasicBlock *BB = Guard->getParent();
-  makeGuardControlFlowExplicit(DeoptIntrinsic, Guard);
-  BranchInst *ExplicitGuard = cast<BranchInst>(BB->getTerminator());
-  assert(ExplicitGuard->isConditional() && "Must be!");
+  makeGuardControlFlowExplicit(DeoptIntrinsic, Guard, true);
+  assert(isWidenableBranch(BB->getTerminator()) && "should hold");
 
-  // We want the guard to be expressed as explicit control flow, but still be
-  // widenable. For that, we add Widenable Condition intrinsic call to the
-  // guard's condition.
-  IRBuilder<> B(ExplicitGuard);
-  auto *WidenableCondition =
-      B.CreateIntrinsic(Intrinsic::experimental_widenable_condition,
-                        {}, {}, nullptr, "widenable_cond");
-  WidenableCondition->setCallingConv(Guard->getCallingConv());
-  auto *NewCond =
-      B.CreateAnd(ExplicitGuard->getCondition(), WidenableCondition);
-  NewCond->setName("exiplicit_guard_cond");
-  ExplicitGuard->setCondition(NewCond);
   Guard->eraseFromParent();
 }
 

diff  --git a/llvm/lib/Transforms/Utils/GuardUtils.cpp b/llvm/lib/Transforms/Utils/GuardUtils.cpp
index 37fca0d129b0..b8c4c764e74a 100644
--- a/llvm/lib/Transforms/Utils/GuardUtils.cpp
+++ b/llvm/lib/Transforms/Utils/GuardUtils.cpp
@@ -26,7 +26,7 @@ static cl::opt<uint32_t> PredicatePassBranchWeight(
              "reciprocal of this value (default = 1 << 20)"));
 
 void llvm::makeGuardControlFlowExplicit(Function *DeoptIntrinsic,
-                                        CallInst *Guard) {
+                                        CallInst *Guard, bool UseWC) {
   OperandBundleDef DeoptOB(*Guard->getOperandBundle(LLVMContext::OB_deopt));
   SmallVector<Value *, 4> Args(std::next(Guard->arg_begin()), Guard->arg_end());
 
@@ -62,6 +62,18 @@ void llvm::makeGuardControlFlowExplicit(Function *DeoptIntrinsic,
 
   DeoptCall->setCallingConv(Guard->getCallingConv());
   DeoptBlockTerm->eraseFromParent();
+
+  if (UseWC) {
+    // We want the guard to be expressed as explicit control flow, but still be
+    // widenable. For that, we add Widenable Condition intrinsic call to the
+    // guard's condition.
+    IRBuilder<> B(CheckBI);
+    auto *WC = B.CreateIntrinsic(Intrinsic::experimental_widenable_condition,
+                                 {}, {}, nullptr, "widenable_cond");
+    CheckBI->setCondition(B.CreateAnd(CheckBI->getCondition(), WC,
+                                      "exiplicit_guard_cond"));
+    assert(isWidenableBranch(CheckBI) && "sanity check");
+  }
 }