[MustExec][NFC] Expose a common LoopSafetyInfo interface

[jdoerfert]

Summary

This patch creates an abstract base class to allow new LoopSafetyInfo
implementations that are not based on the curent LoopSafetyInfo block
coloring. In the process, we added two more specialized methods

`insertInstructionBefore/After`

which provide more information than the current

`insertInstructionTo`

method.

[lebedev.ri]

llvm/include/llvm/Analysis/MustExecute.h

73

const Instruction *PosI) {}

[jdoerfert]

Any objections to this?

llvm/include/llvm/Analysis/MustExecute.h

73

I'll clang-format, I though I did already.

[jdoerfert]

llvm/include/llvm/Analysis/MustExecute.h

73

I could probably make them pure as well.

[lebedev.ri]

llvm/include/llvm/Analysis/MustExecute.h

73

I'm mainly commenting on that extra ;

[lebedev.ri]

I'm sorry, i have never ever touched this area of codebase, i don't believe i can be of any help with this patch set. :(

[jdoerfert]

Make methods pure, clang-format

llvm/include/llvm/Analysis/MustExecute.h

 #include "llvm/IR/Instruction.h"
 
 namespace llvm {
 
 class Instruction;
 class DominatorTree;
 class Loop;
 
+/// The common interface for the loop safety information.
+struct LoopSafetyInfoInterface {
+
+  /// Returns true iff the block \p BB potentially may throw exception. It can
+  /// be false-positive in cases when we want to avoid complex analysis.
+  virtual bool blockMayThrow(const BasicBlock *BB) const = 0;
+
+  /// Returns true iff any block of the loop for which this info is contains an
+  /// instruction that may throw or otherwise exit abnormally.
+  virtual bool anyBlockMayThrow() const = 0;
+
+  /// Computes safety information for a loop checks loop body & header for
+  /// the possibility of may throw exception, it takes LoopSafetyInfo and loop
+  /// as argument. Updates safety information in LoopSafetyInfo argument.
+  /// Note: This is defined to clear and reinitialize an already initialized
+  /// LoopSafetyInfo.  Some callers rely on this fact.
+  virtual void computeLoopSafetyInfo(const Loop *CurLoop) = 0;
+
+  /// Returns true if the instruction in a loop is guaranteed to execute at
+  /// least once (under the assumption that the loop is entered).
+  virtual bool isGuaranteedToExecute(const Instruction &Inst,
+                                     const DominatorTree *DT,
+                                     const Loop *CurLoop) const = 0;
+
+  /// Return true if we must reach the block \p BB under assumption that the
+  /// loop \p CurLoop is entered.
+  virtual bool allLoopPathsLeadToBlock(const Loop *CurLoop,
+                                       const BasicBlock *BB,
+                                       const DominatorTree *DT) const = 0;
+
+  /// Inform the safety info that we are planning to insert a new instruction
+  /// \p Inst before \p PosI. It will make all cache updates to keep it correct
+  /// after this insertion.
+  virtual void insertInstructionBefore(const Instruction *NewI,
+                                       const Instruction *PosI) = 0;
+
+  /// Inform the safety info that we are planning to insert a new instruction
+  /// \p Inst after \p PosI. It will make all cache updates to keep it correct
+  /// after this insertion.
+  virtual void insertInstructionAfter(const Instruction *NewI,
+                                      const Instruction *PosI) = 0;
+
+  /// Inform safety info that we are planning to remove the instruction \p Inst
+  /// from its block. It will make all cache updates to keep it correct after
+  /// this removal.
+  virtual void removeInstruction(const Instruction *Inst) = 0;
+};
+
 /// Captures loop safety information.
 /// It keep information for loop blocks may throw exception or otherwise
 /// exit abnormaly on any iteration of the loop which might actually execute
 /// at runtime.  The primary way to consume this infromation is via
 /// isGuaranteedToExecute below, but some callers bailout or fallback to
 /// alternate reasoning if a loop contains any implicit control flow.
 /// NOTE: LoopSafetyInfo contains cached information regarding loops and their
 /// particular blocks. This information is only dropped on invocation of
 /// computeLoopSafetyInfo. If the loop or any of its block is deleted, or if
 /// any thrower instructions have been added or removed from them, or if the
 /// control flow has changed, or in case of other meaningful modifications, the
 /// LoopSafetyInfo needs to be recomputed. If a meaningful modifications to the
 /// loop were made and the info wasn't recomputed properly, the behavior of all
 /// methods except for computeLoopSafetyInfo is undefined.
-class LoopSafetyInfo {
+class LoopSafetyInfo : public LoopSafetyInfoInterface {
   // Used to update funclet bundle operands.
   DenseMap<BasicBlock *, ColorVector> BlockColors;
 
 protected:
   /// Computes block colors.
   void computeBlockColors(const Loop *CurLoop);
 
 public:
   /// Returns block colors map that is used to update funclet operand bundles.
   const DenseMap<BasicBlock *, ColorVector> &getBlockColors() const;
 
   /// Copy colors of block \p Old into the block \p New.
   void copyColors(BasicBlock *New, BasicBlock *Old);
 
-  /// Returns true iff the block \p BB potentially may throw exception. It can
-  /// be false-positive in cases when we want to avoid complex analysis.
-  virtual bool blockMayThrow(const BasicBlock *BB) const = 0;
-
-  /// Returns true iff any block of the loop for which this info is contains an
-  /// instruction that may throw or otherwise exit abnormally.
-  virtual bool anyBlockMayThrow() const = 0;
-
   /// Return true if we must reach the block \p BB under assumption that the
   /// loop \p CurLoop is entered.
   bool allLoopPathsLeadToBlock(const Loop *CurLoop, const BasicBlock *BB,
-                               const DominatorTree *DT) const;
-
-  /// Computes safety information for a loop checks loop body & header for
-  /// the possibility of may throw exception, it takes LoopSafetyInfo and loop
-  /// as argument. Updates safety information in LoopSafetyInfo argument.
-  /// Note: This is defined to clear and reinitialize an already initialized
-  /// LoopSafetyInfo.  Some callers rely on this fact.
-  virtual void computeLoopSafetyInfo(const Loop *CurLoop) = 0;
-
-  /// Returns true if the instruction in a loop is guaranteed to execute at
-  /// least once (under the assumption that the loop is entered).
-  virtual bool isGuaranteedToExecute(const Instruction &Inst,
-                                     const DominatorTree *DT,
-                                     const Loop *CurLoop) const = 0;
+                               const DominatorTree *DT) const override;
 
   LoopSafetyInfo() = default;
 
   virtual ~LoopSafetyInfo() = default;
 };
 
 
 /// Simple and conservative implementation of LoopSafetyInfo that can give
 /// false-positive answers to its queries in order to avoid complicated
 /// analysis.
-class SimpleLoopSafetyInfo: public LoopSafetyInfo {
+class SimpleLoopSafetyInfo : public LoopSafetyInfo {
   bool MayThrow = false;       // The current loop contains an instruction which
                                // may throw.
   bool HeaderMayThrow = false; // Same as previous, but specific to loop header
 
 public:
   virtual bool blockMayThrow(const BasicBlock *BB) const;
 
   virtual bool anyBlockMayThrow() const;
 
   virtual void computeLoopSafetyInfo(const Loop *CurLoop);
 
   virtual bool isGuaranteedToExecute(const Instruction &Inst,
                                      const DominatorTree *DT,
                                      const Loop *CurLoop) const;
 
+  void insertInstructionBefore(const Instruction *NewI,
+                               const Instruction *PosI) {}
+
+  void insertInstructionAfter(const Instruction *NewI,
+                              const Instruction *PosI) {}
+
+  void removeInstruction(const Instruction *Inst) {}
+
   SimpleLoopSafetyInfo() : LoopSafetyInfo() {};
 
   virtual ~SimpleLoopSafetyInfo() {};
 };
 
 /// This implementation of LoopSafetyInfo use ImplicitControlFlowTracking to
 /// give precise answers on "may throw" queries. This implementation uses cache
 /// that should be invalidated by calling the methods insertInstructionTo and
public:
   bool doesNotWriteMemoryBefore(const Instruction &I, const Loop *CurLoop)
       const;
 
   /// Inform the safety info that we are planning to insert a new instruction
   /// \p Inst into the basic block \p BB. It will make all cache updates to keep
   /// it correct after this insertion.
   void insertInstructionTo(const Instruction *Inst, const BasicBlock *BB);
 
+  /// See LoopSafetyInfoInterface::insertInstructionBefore(...).
+  void insertInstructionBefore(const Instruction *NewI,
+                               const Instruction *PosI) {
+    insertInstructionTo(NewI, PosI->getParent());
+  }
+
+  /// See LoopSafetyInfoInterface::insertInstructionAfter(...).
+  void insertInstructionAfter(const Instruction *NewI,
+                              const Instruction *PosI) {
+    insertInstructionTo(NewI, PosI->getParent());
+  };
+
   /// Inform safety info that we are planning to remove the instruction \p Inst
   /// from its block. It will make all cache updates to keep it correct after
   /// this removal.
   void removeInstruction(const Instruction *Inst);
 
   ICFLoopSafetyInfo(DominatorTree *DT) : LoopSafetyInfo(), ICF(DT), MW(DT) {};
 
   virtual ~ICFLoopSafetyInfo() {};
 };
 
 }
 
 #endif

llvm/lib/Transforms/Scalar/LICM.cpp

for (BasicBlock::iterator II = BB->begin(), E = BB->end(); II != E;) {
       // converting it to a reciprocal multiplication.
       if (I.getOpcode() == Instruction::FDiv &&
           CurLoop->isLoopInvariant(I.getOperand(1)) &&
           I.hasAllowReciprocal()) {
         auto Divisor = I.getOperand(1);
         auto One = llvm::ConstantFP::get(Divisor->getType(), 1.0);
         auto ReciprocalDivisor = BinaryOperator::CreateFDiv(One, Divisor);
         ReciprocalDivisor->setFastMathFlags(I.getFastMathFlags());
-        SafetyInfo->insertInstructionTo(ReciprocalDivisor, I.getParent());
+        SafetyInfo->insertInstructionBefore(ReciprocalDivisor, &I);
         ReciprocalDivisor->insertBefore(&I);
 
         auto Product =
             BinaryOperator::CreateFMul(I.getOperand(0), ReciprocalDivisor);
         Product->setFastMathFlags(I.getFastMathFlags());
-        SafetyInfo->insertInstructionTo(Product, I.getParent());
+        SafetyInfo->insertInstructionAfter(Product, &I);
         Product->insertAfter(&I);
         I.replaceAllUsesWith(Product);
         eraseInstruction(I, *SafetyInfo, CurAST, MSSAU);
 
         hoist(*ReciprocalDivisor, DT, CurLoop, CFH.getOrCreateHoistedBlock(BB),
               SafetyInfo, MSSAU, ORE);
         HoistedInstructions.push_back(ReciprocalDivisor);
         Changed = true;
static void eraseInstruction(Instruction &I, ICFLoopSafetyInfo &SafetyInfo,
   SafetyInfo.removeInstruction(&I);
   I.eraseFromParent();
 }
 
 static void moveInstructionBefore(Instruction &I, Instruction &Dest,
                                   ICFLoopSafetyInfo &SafetyInfo,
                                   MemorySSAUpdater *MSSAU) {
   SafetyInfo.removeInstruction(&I);
-  SafetyInfo.insertInstructionTo(&I, Dest.getParent());
+  SafetyInfo.insertInstructionBefore(&I, &Dest);
   I.moveBefore(&Dest);
   if (MSSAU)
     if (MemoryUseOrDef *OldMemAcc = cast_or_null<MemoryUseOrDef>(
             MSSAU->getMemorySSA()->getMemoryAccess(&I)))
       MSSAU->moveToPlace(OldMemAcc, Dest.getParent(), MemorySSA::End);
 }
 
 static Instruction *sinkThroughTriviallyReplaceablePHI(