[LoopInfo] SFINAE mechanism for hoist into check

[Dinistro]

Summary

This commit introduces an SFINAE mechanism to make the LLVM hoist into
check member function not leak into the users of LoopInfo.

Depends on D148235

[Dinistro]

fix tests by querying for a non-static member function

[Dinistro]

rebased

[Dinistro]

rebased. CI will still break due to bug in main

[Dinistro]

This is now ready for review. Note that the CI fails because of an unrelated ORC issue, that is present in main as well.

[zero9178]

llvm/include/llvm/Support/GenericLoopInfoImpl.h

185

Somewhat small nit, but this can probably be implemented nicer using if constexpr

template <class BlockT>
bool isLegalToHoistInto(BlockT *Block) {
   if constexpr (detect_has_hoist_check<BlockT>::value)
       return Block->isLegalToHoistInto();
   else
       return false;
}

Less SFINAE magic!

[Dinistro]

address review comments

[Dinistro]

llvm/include/llvm/Support/GenericLoopInfoImpl.h

185

I forgot about the if constexpr construct, thanks for the suggestion.

llvm/include/llvm/Support/GenericLoopInfoImpl.h

void LoopBase<BlockT, LoopT>::getExitEdges(
   assert(!isInvalid() && "Loop not in a valid state!");
   for (const auto BB : blocks())
     for (auto *Succ : children<BlockT *>(BB))
       if (!contains(Succ))
         // Not in current loop? It must be an exit block.
         ExitEdges.emplace_back(BB, Succ);
 }
 
+namespace detail {
+template <class BlockT>
+using has_hoist_check = decltype(&BlockT::isLegalToHoistInto);
+
+template <class BlockT>
+using detect_has_hoist_check = llvm::is_detected<has_hoist_check, BlockT>;
+
+/// SFINAE functions that dispatch to the isLegalToHoistInto member function or
+/// return false, if it doesn't exist.
+template <class BlockT>
+std::enable_if_t<detect_has_hoist_check<BlockT>::value, bool>
+isLegalToHoistInto(BlockT *Block) {
+  return Block->isLegalToHoistInto();
+}
+template <class BlockT>
+std::enable_if_t<!detect_has_hoist_check<BlockT>::value, bool>
+isLegalToHoistInto(BlockT *) {
+  return false;
+}
+} // namespace detail
+
 /// getLoopPreheader - If there is a preheader for this loop, return it.  A
 /// loop has a preheader if there is only one edge to the header of the loop
 /// from outside of the loop and it is legal to hoist instructions into the
 /// predecessor. If this is the case, the block branching to the header of the
 /// loop is the preheader node.
 ///
 /// This method returns null if there is no preheader for the loop.
 ///
 template <class BlockT, class LoopT>
 BlockT *LoopBase<BlockT, LoopT>::getLoopPreheader() const {
   assert(!isInvalid() && "Loop not in a valid state!");
   // Keep track of nodes outside the loop branching to the header...
   BlockT *Out = getLoopPredecessor();
   if (!Out)
     return nullptr;
 
   // Make sure we are allowed to hoist instructions into the predecessor.
-  if (!Out->isLegalToHoistInto())
+  if (!detail::isLegalToHoistInto(Out))
     return nullptr;
 
   // Make sure there is only one exit out of the preheader.
   typedef GraphTraits<BlockT *> BlockTraits;
   typename BlockTraits::ChildIteratorType SI = BlockTraits::child_begin(Out);
   ++SI;
   if (SI != BlockTraits::child_end(Out))
     return nullptr; // Multiple exits from the block, must not be a preheader.

mlir/include/mlir/IR/Block.h

public:
   void dump();
 
   /// Print out the name of the block without printing its body.
   /// NOTE: The printType argument is ignored.  We keep it for compatibility
   /// with LLVM dominator machinery that expects it to exist.
   void printAsOperand(raw_ostream &os, bool printType = true);
   void printAsOperand(raw_ostream &os, AsmState &state);
 
-  /// NOTE: Do not call this function, it is only used to be compatible with the
-  /// LLVM loop analysis machinery.
-  bool isLegalToHoistInto() const { return false; };
-
 private:
   /// Pair of the parent object that owns this block and a bit that signifies if
   /// the operations within this block have a valid ordering.
   llvm::PointerIntPair<Region *, /*IntBits=*/1, bool> parentValidOpOrderPair;
 
   /// This is the list of operations in the block.
   OpListType operations;