[Polly][ManualOpt] Match interpretation of unroll metadata to LoopUnrolls's.

We previously had a different interpretation of unroll transformation
attributes than how LoopUnroll interpreted it. In particular,
llvm.loop.unroll.enable was needed explicitly to enable it and disabling
metadata was ignored.
Additionally, it required that either full unrolling or an unroll factor
to be specified or fail otherwise. An unroll factor is still required,
but the transformation is ignored with the hope that LoopUnroll is going
to apply the unrolling, since Polly currently does not implement an
heuristic.

Fixes llvm.org/PR50109

polly/include/polly/Support/ScopHelper.h

@@ -562,6 +562,14 @@ bool hasDebugCall(ScopStmt *Stmt);
 llvm::Optional<llvm::Metadata *> findMetadataOperand(llvm::MDNode *LoopMD,
                                                      llvm::StringRef Name);
 
+/// Find a boolean property value in a LoopID. The value not being defined is
+/// interpreted as a false value.
+bool getBooleanLoopAttribute(llvm::MDNode *LoopID, llvm::StringRef Name);
+
+/// Find an integers property value in a LoopID.
+llvm::Optional<int> getOptionalIntLoopAttribute(llvm::MDNode *LoopID,
+                                                llvm::StringRef Name);
+
 /// Does the loop's LoopID contain a 'llvm.loop.disable_heuristics' property?
 ///
 /// This is equivalent to llvm::hasDisableAllTransformsHint(Loop*), but
@@ -569,6 +577,7 @@ llvm::Optional<llvm::Metadata *> findMetadataOperand(llvm::MDNode *LoopMD,
 /// which clashes with polly::MemoryAccess. Declaring this alias here avoid
 /// having to include LoopUtils.h in other files.
 bool hasDisableAllTransformsHint(llvm::Loop *L);
+bool hasDisableAllTransformsHint(llvm::MDNode *LoopID);
 
 /// Represent the attributes of a loop.
 struct BandAttr {

polly/lib/Support/ScopHelper.cpp

@@ -746,6 +746,21 @@ static MDNode *findNamedMetadataNode(MDNode *LoopMD, StringRef Name) {
   return nullptr;
 }
 
+static Optional<const MDOperand *> findNamedMetadataArg(MDNode *LoopID,
+                                                        StringRef Name) {
+  MDNode *MD = findNamedMetadataNode(LoopID, Name);
+  if (!MD)
+    return None;
+  switch (MD->getNumOperands()) {
+  case 1:
+    return nullptr;
+  case 2:
+    return &MD->getOperand(1);
+  default:
+    llvm_unreachable("loop metadata has 0 or 1 operand");
+  }
+}
+
 Optional<Metadata *> polly::findMetadataOperand(MDNode *LoopMD,
                                                 StringRef Name) {
   MDNode *MD = findNamedMetadataNode(LoopMD, Name);
@@ -761,10 +776,49 @@ Optional<Metadata *> polly::findMetadataOperand(MDNode *LoopMD,
   }
 }
 
+static Optional<bool> getOptionalBoolLoopAttribute(MDNode *LoopID,
+                                                   StringRef Name) {
+  MDNode *MD = findNamedMetadataNode(LoopID, Name);
+  if (!MD)
+    return None;
+  switch (MD->getNumOperands()) {
+  case 1:
+    return true;
+  case 2:
+    if (ConstantInt *IntMD =
+            mdconst::extract_or_null<ConstantInt>(MD->getOperand(1).get()))
+      return IntMD->getZExtValue();
+    return true;
+  }
+  llvm_unreachable("unexpected number of options");
+}
+
+bool polly::getBooleanLoopAttribute(MDNode *LoopID, StringRef Name) {
+  return getOptionalBoolLoopAttribute(LoopID, Name).getValueOr(false);
+}
+
+llvm::Optional<int> polly::getOptionalIntLoopAttribute(MDNode *LoopID,
+                                                       StringRef Name) {
+  const MDOperand *AttrMD =
+      findNamedMetadataArg(LoopID, Name).getValueOr(nullptr);
+  if (!AttrMD)
+    return None;
+
+  ConstantInt *IntMD = mdconst::extract_or_null<ConstantInt>(AttrMD->get());
+  if (!IntMD)
+    return None;
+
+  return IntMD->getSExtValue();
+}
+
 bool polly::hasDisableAllTransformsHint(Loop *L) {
   return llvm::hasDisableAllTransformsHint(L);
 }
 
+bool polly::hasDisableAllTransformsHint(llvm::MDNode *LoopID) {
+  return getBooleanLoopAttribute(LoopID, "llvm.loop.disable_nonforced");
+}
+
 isl::id polly::getIslLoopAttr(isl::ctx Ctx, BandAttr *Attr) {
   assert(Attr && "Must be a valid BandAttr");
 

polly/lib/Transform/ManualOptimizer.cpp

@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/IR/Metadata.h"
+#include "llvm/Transforms/Utils/LoopUtils.h"
 
 #define DEBUG_TYPE "polly-opt-manual"
 
@@ -24,42 +25,36 @@ using namespace polly;
 using namespace llvm;
 
 namespace {
-/// Extract an integer property from an LoopID metadata node.
-static llvm::Optional<int64_t> findOptionalIntOperand(MDNode *LoopMD,
-                                                      StringRef Name) {
-  Metadata *AttrMD = findMetadataOperand(LoopMD, Name).getValueOr(nullptr);
-  if (!AttrMD)
-    return None;
+/// Same as llvm::hasUnrollTransformation(), but takes a LoopID as argument
+/// instead of a Loop.
+static TransformationMode hasUnrollTransformation(MDNode *LoopID) {
+  if (getBooleanLoopAttribute(LoopID, "llvm.loop.unroll.disable"))
+    return TM_SuppressedByUser;
 
-  ConstantInt *IntMD = mdconst::extract_or_null<ConstantInt>(AttrMD);
-  if (!IntMD)
-    return None;
+  Optional<int> Count =
+      getOptionalIntLoopAttribute(LoopID, "llvm.loop.unroll.count");
+  if (Count.hasValue())
+    return Count.getValue() == 1 ? TM_SuppressedByUser : TM_ForcedByUser;
 
-  return IntMD->getSExtValue();
-}
+  if (getBooleanLoopAttribute(LoopID, "llvm.loop.unroll.enable"))
+    return TM_ForcedByUser;
 
-/// Extract boolean property from an LoopID metadata node.
-static llvm::Optional<bool> findOptionalBoolOperand(MDNode *LoopMD,
-                                                    StringRef Name) {
-  auto MD = findOptionMDForLoopID(LoopMD, Name);
-  if (!MD)
-    return None;
+  if (getBooleanLoopAttribute(LoopID, "llvm.loop.unroll.full"))
+    return TM_ForcedByUser;
 
-  switch (MD->getNumOperands()) {
-  case 1:
-    // When the value is absent it is interpreted as 'attribute set'.
-    return true;
-  case 2:
-    ConstantInt *IntMD =
-        mdconst::extract_or_null<ConstantInt>(MD->getOperand(1).get());
-    return IntMD->getZExtValue() != 0;
-  }
-  llvm_unreachable("unexpected number of options");
+  if (hasDisableAllTransformsHint(LoopID))
+    return TM_Disable;
+
+  return TM_Unspecified;
 }
 
 /// Apply full or partial unrolling.
 static isl::schedule applyLoopUnroll(MDNode *LoopMD,
                                      isl::schedule_node BandToUnroll) {
+  TransformationMode UnrollMode = ::hasUnrollTransformation(LoopMD);
+  if (UnrollMode & TM_Disable)
+    return {};
+
   assert(BandToUnroll);
   // TODO: Isl's codegen also supports unrolling by isl_ast_build via
   // isl_schedule_node_band_set_ast_build_options({ unroll[x] }) which would be
@@ -67,10 +62,9 @@ static isl::schedule applyLoopUnroll(MDNode *LoopMD,
   // unrolled loop could be input of another loop transformation which expects
   // the explicit schedule nodes. That is, we would need this explicit expansion
   // anyway and using the ISL codegen option is a compile-time optimization.
-  int64_t Factor =
-      findOptionalIntOperand(LoopMD, "llvm.loop.unroll.count").getValueOr(0);
-  bool Full = findOptionalBoolOperand(LoopMD, "llvm.loop.unroll.full")
-                  .getValueOr(false);
+  int64_t Factor = getOptionalIntLoopAttribute(LoopMD, "llvm.loop.unroll.count")
+                       .getValueOr(0);
+  bool Full = getBooleanLoopAttribute(LoopMD, "llvm.loop.unroll.full");
   assert((!Full || !(Factor > 0)) &&
          "Cannot unroll fully and partially at the same time");
 
@@ -80,7 +74,8 @@ static isl::schedule applyLoopUnroll(MDNode *LoopMD,
   if (Factor > 0)
     return applyPartialUnroll(BandToUnroll, Factor);
 
-  llvm_unreachable("Negative unroll factor");
+  // For heuristic unrolling, fall back to LLVM's LoopUnroll pass.
+  return {};
 }
 
 // Return the properties from a LoopID. Scalar properties are ignored.
@@ -143,16 +138,18 @@ public:
         continue;
       StringRef AttrName = NameMD->getString();
 
-      if (AttrName == "llvm.loop.unroll.enable") {
-        // TODO: Handle disabling like llvm::hasUnrollTransformation().
+      // Honor transformation order; transform the first transformation in the
+      // list first.
+      if (AttrName == "llvm.loop.unroll.enable" ||
+          AttrName == "llvm.loop.unroll.count" ||
+          AttrName == "llvm.loop.unroll.full") {
         Result = applyLoopUnroll(LoopMD, Band);
-      } else {
-        // not a loop transformation; look for next property
-        continue;
+        if (Result)
+          return;
       }
 
-      assert(Result && "expecting applied transformation");
-      return;
+      // not a loop transformation; look for next property
+      continue;
     }
   }
 

polly/test/ScheduleOptimizer/ManualOptimization/unroll_disable.ll

@@ -0,0 +1,37 @@
+; RUN: opt %loadPolly -polly-opt-isl -polly-pragma-based-opts=1 -analyze < %s | FileCheck %s --match-full-lines
+;
+; Override unroll metadata with llvm.loop.unroll.disable.
+;
+define void @func(i32 %n, double* noalias nonnull %A) {
+entry:
+  br label %for
+
+for:
+  %j = phi i32 [0, %entry], [%j.inc, %inc]
+  %j.cmp = icmp slt i32 %j, %n
+  br i1 %j.cmp, label %body, label %exit
+
+    body:
+      store double 42.0, double* %A
+      br label %inc
+
+inc:
+  %j.inc = add nuw nsw i32 %j, 1
+  br label %for, !llvm.loop !2
+
+exit:
+  br label %return
+
+return:
+  ret void
+}
+
+
+!2 = distinct !{!2, !3, !4}
+!3 = !{!"llvm.loop.unroll.count", i32 4}
+!4 = !{!"llvm.loop.unroll.disable"}
+
+
+; CHECK-LABEL: Printing analysis 'Polly - Optimize schedule of SCoP' for region: 'for => return' in function 'func':
+; CHECK-NEXT:  Calculated schedule:
+; CHECK-NEXT:    n/a

polly/test/ScheduleOptimizer/ManualOptimization/unroll_full.ll

@@ -27,8 +27,7 @@ return:
 }
 
 
-!2 = distinct !{!2, !4, !5}
-!4 = !{!"llvm.loop.unroll.enable", i1 true}
+!2 = distinct !{!2, !5}
 !5 = !{!"llvm.loop.unroll.full"}
 
 

polly/test/ScheduleOptimizer/ManualOptimization/unroll_heuristic.ll

@@ -0,0 +1,38 @@
+; RUN: opt %loadPolly -polly-opt-isl -polly-pragma-based-opts=1 -analyze < %s | FileCheck %s --match-full-lines
+; RUN: opt %loadPolly -polly-opt-isl -polly-pragma-based-opts=0 -analyze < %s | FileCheck %s --match-full-lines
+;
+; Unrolling with heuristic factor.
+; Currently not supported and expected to be handled by LLVM's unroll pass.
+;
+define void @func(i32 %n, double* noalias nonnull %A) {
+entry:
+  br label %for
+
+for:
+  %j = phi i32 [0, %entry], [%j.inc, %inc]
+  %j.cmp = icmp slt i32 %j, %n
+  br i1 %j.cmp, label %body, label %exit
+
+    body:
+      store double 42.0, double* %A
+      br label %inc
+
+inc:
+  %j.inc = add nuw nsw i32 %j, 1
+  br label %for, !llvm.loop !2
+
+exit:
+  br label %return
+
+return:
+  ret void
+}
+
+
+!2 = distinct !{!2, !4}
+!4 = !{!"llvm.loop.unroll.enable", i1 true}
+
+
+; CHECK-LABEL: Printing analysis 'Polly - Optimize schedule of SCoP' for region: 'for => return' in function 'func':
+; CHECK-NEXT:  Calculated schedule:
+; CHECK-NEXT:    n/a

polly/test/ScheduleOptimizer/ManualOptimization/unroll_partial.ll

@@ -28,8 +28,7 @@ return:
 }
 
 
-!2 = distinct !{!2, !4, !5}
-!4 = !{!"llvm.loop.unroll.enable", i1 true}
+!2 = distinct !{!2, !5}
 !5 = !{!"llvm.loop.unroll.count", i4 4}