[llvm] f96cb66 - [ValueTracking] Accept Instruction in isSafeToSpeculativelyExecute() (NFC)

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2022-07-06T11:12:49+02:00
New Revision: f96cb66d1931dc09be947502a3f1dccda933cc1b

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

LOG: [ValueTracking] Accept Instruction in isSafeToSpeculativelyExecute() (NFC)

As constant expressions can no longer trap, it only makes sense to
call isSafeToSpeculativelyExecute on Instructions, so limit the
API to accept only them, rather than general Operators or Values.

Added: 
    

Modified: 
    llvm/include/llvm/Analysis/ValueTracking.h
    llvm/lib/Analysis/ValueTracking.cpp
    llvm/lib/CodeGen/ExpandVectorPredication.cpp
    llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index 3b29bf1d53b47..7cb1a91d8c936 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -458,7 +458,7 @@ constexpr unsigned MaxAnalysisRecursionDepth = 6;
   ///
   /// This method can return true for instructions that read memory;
   /// for such instructions, moving them may change the resulting value.
-  bool isSafeToSpeculativelyExecute(const Value *V,
+  bool isSafeToSpeculativelyExecute(const Instruction *I,
                                     const Instruction *CtxI = nullptr,
                                     const DominatorTree *DT = nullptr,
                                     const TargetLibraryInfo *TLI = nullptr);
@@ -481,8 +481,8 @@ constexpr unsigned MaxAnalysisRecursionDepth = 6;
   ///   This behavior is a shortcoming in the current implementation and not
   ///   intentional.
   bool isSafeToSpeculativelyExecuteWithOpcode(
-      unsigned Opcode, const Operator *Inst, const Instruction *CtxI = nullptr,
-      const DominatorTree *DT = nullptr,
+      unsigned Opcode, const Instruction *Inst,
+      const Instruction *CtxI = nullptr, const DominatorTree *DT = nullptr,
       const TargetLibraryInfo *TLI = nullptr);
 
   /// Returns true if the result or effects of the given instructions \p I

diff  --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 0e8c190cc9af0..add2d427e05b7 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -4679,27 +4679,22 @@ bool llvm::mustSuppressSpeculation(const LoadInst &LI) {
     F.hasFnAttribute(Attribute::SanitizeHWAddress);
 }
 
-
-bool llvm::isSafeToSpeculativelyExecute(const Value *V,
+bool llvm::isSafeToSpeculativelyExecute(const Instruction *Inst,
                                         const Instruction *CtxI,
                                         const DominatorTree *DT,
                                         const TargetLibraryInfo *TLI) {
-  const Operator *Inst = dyn_cast<Operator>(V);
-  if (!Inst)
-    return false;
-  return isSafeToSpeculativelyExecuteWithOpcode(Inst->getOpcode(), Inst, CtxI, DT, TLI);
+  return isSafeToSpeculativelyExecuteWithOpcode(Inst->getOpcode(), Inst, CtxI,
+                                                DT, TLI);
 }
 
-bool llvm::isSafeToSpeculativelyExecuteWithOpcode(unsigned Opcode,
-                                        const Operator *Inst,
-                                        const Instruction *CtxI,
-                                        const DominatorTree *DT,
-                                        const TargetLibraryInfo *TLI) {
+bool llvm::isSafeToSpeculativelyExecuteWithOpcode(
+    unsigned Opcode, const Instruction *Inst, const Instruction *CtxI,
+    const DominatorTree *DT, const TargetLibraryInfo *TLI) {
 #ifndef NDEBUG
   if (Inst->getOpcode() != Opcode) {
     // Check that the operands are actually compatible with the Opcode override.
     auto hasEqualReturnAndLeadingOperandTypes =
-        [](const Operator *Inst, unsigned NumLeadingOperands) {
+        [](const Instruction *Inst, unsigned NumLeadingOperands) {
           if (Inst->getNumOperands() < NumLeadingOperands)
             return false;
           const Type *ExpectedType = Inst->getType();

diff  --git a/llvm/lib/CodeGen/ExpandVectorPredication.cpp b/llvm/lib/CodeGen/ExpandVectorPredication.cpp
index 7883a48d121c4..59932a542bbc9 100644
--- a/llvm/lib/CodeGen/ExpandVectorPredication.cpp
+++ b/llvm/lib/CodeGen/ExpandVectorPredication.cpp
@@ -120,8 +120,7 @@ static bool maySpeculateLanes(VPIntrinsic &VPI) {
   // Fallback to whether the intrinsic is speculatable.
   Optional<unsigned> OpcOpt = VPI.getFunctionalOpcode();
   unsigned FunctionalOpc = OpcOpt.value_or((unsigned)Instruction::Call);
-  return isSafeToSpeculativelyExecuteWithOpcode(FunctionalOpc,
-                                                cast<Operator>(&VPI));
+  return isSafeToSpeculativelyExecuteWithOpcode(FunctionalOpc, &VPI);
 }
 
 //// } Helpers

diff  --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 322046e7ef6f0..86b2128a19374 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -377,7 +377,8 @@ static void AddPredecessorToBlock(BasicBlock *Succ, BasicBlock *NewPred,
 /// expensive.
 static InstructionCost computeSpeculationCost(const User *I,
                                               const TargetTransformInfo &TTI) {
-  assert(isSafeToSpeculativelyExecute(I) &&
+  assert((!isa<Instruction>(I) ||
+          isSafeToSpeculativelyExecute(cast<Instruction>(I))) &&
          "Instruction is not safe to speculatively execute!");
   return TTI.getUserCost(I, TargetTransformInfo::TCK_SizeAndLatency);
 }
@@ -1603,11 +1604,6 @@ bool SimplifyCFGOpt::HoistThenElseCodeToIf(BranchInst *BI,
       if (passingValueIsAlwaysUndefined(BB1V, &PN) ||
           passingValueIsAlwaysUndefined(BB2V, &PN))
         return Changed;
-
-      if (isa<ConstantExpr>(BB1V) && !isSafeToSpeculativelyExecute(BB1V))
-        return Changed;
-      if (isa<ConstantExpr>(BB2V) && !isSafeToSpeculativelyExecute(BB2V))
-        return Changed;
     }
   }