[PATCH] D148504: [LoopInfo] SFINAE mechanism for hoist into check

Christian Ulmann via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 19 01:39:08 PDT 2023


Dinistro created this revision.
Herald added subscribers: bviyer, Moerafaat, zero9178, bzcheeseman, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagainst, shauheen, rriddle, mehdi_amini, thopre.
Herald added a reviewer: rriddle.
Herald added a project: All.
Dinistro updated this revision to Diff 514167.
Dinistro added a comment.
Dinistro updated this revision to Diff 514534.
Dinistro updated this revision to Diff 514862.
Dinistro added a reviewer: ftynse.
Dinistro published this revision for review.
Herald added subscribers: llvm-commits, stephenneuendorffer, nicolasvasilache.
Herald added projects: MLIR, LLVM.

fix tests by querying for a non-static member function


Dinistro added a comment.

rebased


Dinistro added a comment.

rebased. CI will still break due to bug in main


Dinistro added a comment.

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


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 <https://reviews.llvm.org/D148235>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148504

Files:
  llvm/include/llvm/Support/GenericLoopInfoImpl.h
  mlir/include/mlir/IR/Block.h


Index: mlir/include/mlir/IR/Block.h
===================================================================
--- mlir/include/mlir/IR/Block.h
+++ mlir/include/mlir/IR/Block.h
@@ -351,10 +351,6 @@
   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.
Index: llvm/include/llvm/Support/GenericLoopInfoImpl.h
===================================================================
--- llvm/include/llvm/Support/GenericLoopInfoImpl.h
+++ llvm/include/llvm/Support/GenericLoopInfoImpl.h
@@ -171,6 +171,27 @@
         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
@@ -188,7 +209,7 @@
     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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D148504.514862.patch
Type: text/x-patch
Size: 2119 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230419/eef819e4/attachment.bin>


More information about the llvm-commits mailing list