Add new llvm.loop.unroll.enable metadata which is now generated by "#pragma unroll" directive

[meheff]

Summary

This change adds the new unroll metadata "llvm.loop.unroll.enable" which directs
the optimizer to unroll a loop fully if the trip count is known at compile time, and
unroll partially if the trip count is not known at compile time. This differs from
"llvm.loop.unroll.full" which explicitly does not unroll a loop if the trip count is not
known at compile time

With this change "#pragma unroll" generates "llvm.loop.unroll.enable" rather than
"llvm.loop.unroll.full" metadata. This changes the semantics of "#pragma unroll" slightly
to mean "unroll aggressively (fully or partially)" rather than "unroll fully or not at all".

The motivating example for this change was some internal code with a loop marked
with "#pragma unroll" which only sometimes had a compile-time trip count depending
on template magic. When the trip count was a compile-time constant, everything works
as expected and the loop is fully unrolled. However, when the trip count was not a
compile-time constant the "#pragma unroll" explicitly disabled unrolling of the loop(!).
Removing "#pragma unroll" caused the loop to be unrolled partially which was desirable
from a performance perspective.

Optimizer patch will follow shortly.

[tyler.nowicki]

Hi Mark,

The patch LGTM!

Tyler

[meheff]

Ping, Hal? Thanks!

[hfinkel]

LGTM, but obviously any necessary CodeGen changes must be done first (or at the same time).

[meheff]

Thanks!

docs/LanguageExtensions.rst

 
 Unrolling a loop reduces the loop control overhead and exposes more
 opportunities for ILP. Loops can be fully or partially unrolled. Full unrolling
 eliminates the loop and replaces it with an enumerated sequence of loop
 iterations. Full unrolling is only possible if the loop trip count is known at
 compile time. Partial unrolling replicates the loop body within the loop and
 reduces the trip count.
 
-If ``unroll(full)`` is specified the unroller will attempt to fully unroll the
+If ``unroll(enable)`` is specified the unroller will attempt to fully unroll the
 loop if the trip count is known at compile time. If the fully unrolled code size
 is greater than an internal limit the loop will be partially unrolled up to this
-limit. If the loop count is not known at compile time the loop will not be
-unrolled.
+limit. If the trip count is not known at compile time the loop will be partially
+unrolled with a heuristically chosen unroll factor.
+
+.. code-block:: c++
+
+  #pragma clang loop unroll(enable)
+  for(...) {
+    ...
+  }
+
+If ``unroll(full)`` is specified the unroller will attempt to fully unroll the
+loop if the trip count is known at compile time identically to
+``unroll(enable)``. However, with ``unroll(full)`` the loop will not be unrolled
+if the loop count is not known at compile time.
 
 .. code-block:: c++
 
   #pragma clang loop unroll(full)
   for(...) {
     ...
   }
 
 The unroll count can be specified explicitly with ``unroll_count(_value_)`` where
 _value_ is a positive integer. If this value is greater than the trip count the
 loop will be fully unrolled. Otherwise the loop is partially unrolled subject
-to the same code size limit as with ``unroll(full)``.
+to the same code size limit as with ``unroll(enable)``.
 
 .. code-block:: c++
 
   #pragma clang loop unroll_count(8)
   for(...) {
     ...
   }
 

include/clang/Basic/Attr.td

def LoopHint : Attr {
 
   /// State of the loop optimization specified by the spelling.
   let Args = [EnumArgument<"Option", "OptionType",
                           ["vectorize", "vectorize_width", "interleave", "interleave_count",
                            "unroll", "unroll_count"],
                           ["Vectorize", "VectorizeWidth", "Interleave", "InterleaveCount",
                            "Unroll", "UnrollCount"]>,
               EnumArgument<"State", "LoopHintState",
-                           ["default", "enable", "disable", "assume_safety"],
-                           ["Default", "Enable", "Disable", "AssumeSafety"]>,
+                           ["enable", "disable", "numeric", "assume_safety", "full"],
+                           ["Enable", "Disable", "Numeric", "AssumeSafety", "Full"]>,
               ExprArgument<"Value">];
 
   let AdditionalMembers = [{
   static const char *getOptionName(int Option) {
     switch(Option) {
     case Vectorize: return "vectorize";
     case VectorizeWidth: return "vectorize_width";
     case Interleave: return "interleave";
def LoopHint : Attr {
   }
 
   // Return a string containing the loop hint argument including the
   // enclosing parentheses.
   std::string getValueString(const PrintingPolicy &Policy) const {
     std::string ValueName;
     llvm::raw_string_ostream OS(ValueName);
     OS << "(";
-    if (option == VectorizeWidth || option == InterleaveCount ||
-        option == UnrollCount)
+    if (state == Numeric)
       value->printPretty(OS, nullptr, Policy);
-    else if (state == Default)
-      return "";
     else if (state == Enable)
-      OS << (option == Unroll ? "full" : "enable");
+      OS << "enable";
+    else if (state == Full)
+      OS << "full";
     else if (state == AssumeSafety)
       OS << "assume_safety";
     else
       OS << "disable";
     OS << ")";
     return OS.str();
   }
 
   // Return a string suitable for identifying this attribute in diagnostics.
   std::string getDiagnosticName(const PrintingPolicy &Policy) const {
     unsigned SpellingIndex = getSpellingListIndex();
     if (SpellingIndex == Pragma_nounroll)
       return "#pragma nounroll";
     else if (SpellingIndex == Pragma_unroll)
-      return "#pragma unroll" + getValueString(Policy);
+      return "#pragma unroll" + (option == UnrollCount ? getValueString(Policy) : "");
 
     assert(SpellingIndex == Pragma_clang_loop && "Unexpected spelling");
     return getOptionName(option) + getValueString(Policy);
   }
   }];
 
   let Documentation = [LoopHintDocs, UnrollHintDocs];
 }

include/clang/Basic/AttrDocs.td

def UnrollHintDocs : Documentation {
   let Category = DocCatStmt;
   let Heading = "#pragma unroll, #pragma nounroll";
   let Content = [{
 Loop unrolling optimization hints can be specified with ``#pragma unroll`` and
 ``#pragma nounroll``. The pragma is placed immediately before a for, while,
 do-while, or c++11 range-based for loop.
 
 Specifying ``#pragma unroll`` without a parameter directs the loop unroller to
-attempt to fully unroll the loop if the trip count is known at compile time:
+attempt to fully unroll the loop if the trip count is known at compile time and
+attempt to partially unroll the loop if the trip count is not known at compile
+time:
 
 .. code-block:: c++
 
   #pragma unroll
   for (...) {
     ...
   }
 

include/clang/Basic/DiagnosticParseKinds.td

 def err_omp_immediate_directive : Error<
   "'#pragma omp %0' cannot be an immediate substatement">;
 def err_omp_expected_identifier_for_critical : Error<
   "expected identifier specifying the name of the 'omp critical' directive">;
 
 // Pragma loop support.
 def err_pragma_loop_missing_argument : Error<
   "missing argument; expected %select{an integer value|"
-  "%select{'enable', 'assume_safety'|'full'}1 or 'disable'}0">;
+  "'enable', %select{'assume_safety'|'full'}1 or 'disable'}0">;
 def err_pragma_loop_invalid_option : Error<
   "%select{invalid|missing}0 option%select{ %1|}0; expected vectorize, "
   "vectorize_width, interleave, interleave_count, unroll, or unroll_count">;
 def err_pragma_invalid_keyword : Error<
-  "invalid argument; expected %select{'enable', 'assume_safety'|'full'}0 or 'disable'">;
+  "invalid argument; expected 'enable', %select{'assume_safety'|'full'}0 or 'disable'">;
 
 // Pragma unroll support.
 def warn_pragma_unroll_cuda_value_in_parens : Warning<
   "argument to '#pragma unroll' should not be in parentheses in CUDA C/C++">,
   InGroup<CudaCompat>;
 } // end of Parse Issue category.
 
 let CategoryName = "Modules Issue" in {

lib/CodeGen/CGLoopInfo.h

 struct LoopAttributes {
   explicit LoopAttributes(bool IsParallel = false);
   void clear();
 
   /// \brief Generate llvm.loop.parallel metadata for loads and stores.
   bool IsParallel;
 
   /// \brief State of loop vectorization or unrolling.
-  enum LVEnableState { Unspecified, Enable, Disable };
+  enum LVEnableState { Unspecified, Enable, Disable, Full };
 
   /// \brief Value for llvm.loop.vectorize.enable metadata.
   LVEnableState VectorizeEnable;
 
-  /// \brief Selects no metadata, llvm.unroll.full, or llvm.unroll.disable.
+  /// \brief Value for llvm.loop.unroll.* metadata (enable, disable, or full).
   LVEnableState UnrollEnable;
 
   /// \brief Value for llvm.loop.vectorize.width metadata.
   unsigned VectorizeWidth;
 
   /// \brief Value for llvm.loop.interleave.count metadata.
   unsigned InterleaveCount;
 
public:
 
   /// \brief Set the next pushed loop 'vectorize.enable'
   void setVectorizeEnable(bool Enable = true) {
     StagedAttrs.VectorizeEnable =
         Enable ? LoopAttributes::Enable : LoopAttributes::Disable;
   }
 
   /// \brief Set the next pushed loop unroll state.
-  void setUnrollEnable(bool Enable = true) {
-    StagedAttrs.UnrollEnable =
-        Enable ? LoopAttributes::Enable : LoopAttributes::Disable;
+  void setUnrollState(const LoopAttributes::LVEnableState &State) {
+    StagedAttrs.UnrollEnable = State;
   }
 
   /// \brief Set the vectorize width for the next loop pushed.
   void setVectorizeWidth(unsigned W) { StagedAttrs.VectorizeWidth = W; }
 
   /// \brief Set the interleave count for the next loop pushed.
   void setInterleaveCount(unsigned C) { StagedAttrs.InterleaveCount = C; }
 

lib/CodeGen/CGLoopInfo.cpp

Metadata *Vals[] = {MDString::get(Ctx, "llvm.loop.vectorize.enable"),
                         ConstantAsMetadata::get(ConstantInt::get(
                             Type::getInt1Ty(Ctx), (Attrs.VectorizeEnable ==
                                                    LoopAttributes::Enable)))};
     Args.push_back(MDNode::get(Ctx, Vals));
   }
 
   // Setting unroll.full or unroll.disable
   if (Attrs.UnrollEnable != LoopAttributes::Unspecified) {
-    Metadata *Vals[] = {
-        MDString::get(Ctx, (Attrs.UnrollEnable == LoopAttributes::Enable
-                                ? "llvm.loop.unroll.full"
-                                : "llvm.loop.unroll.disable"))};
+    std::string Name;
+    if (Attrs.UnrollEnable == LoopAttributes::Enable)
+      Name = "llvm.loop.unroll.enable";
+    else if (Attrs.UnrollEnable == LoopAttributes::Full)
+      Name = "llvm.loop.unroll.full";
+    else
+      Name = "llvm.loop.unroll.disable";
+    Metadata *Vals[] = {MDString::get(Ctx, Name)};
     Args.push_back(MDNode::get(Ctx, Vals));
   }
 
   // Set the first operand to itself.
   MDNode *LoopID = MDNode::get(Ctx, Args);
   LoopID->replaceOperandWith(0, LoopID);
   return LoopID;
 }
case LoopHintAttr::Disable:
         // Disable vectorization by specifying a width of 1.
         setVectorizeWidth(1);
         break;
       case LoopHintAttr::Interleave:
         // Disable interleaving by speciyfing a count of 1.
         setInterleaveCount(1);
         break;
       case LoopHintAttr::Unroll:
-        setUnrollEnable(false);
+        setUnrollState(LoopAttributes::Disable);
         break;
       case LoopHintAttr::UnrollCount:
       case LoopHintAttr::VectorizeWidth:
       case LoopHintAttr::InterleaveCount:
         llvm_unreachable("Options cannot be disabled.");
         break;
       }
       break;
     case LoopHintAttr::Enable:
       switch (Option) {
       case LoopHintAttr::Vectorize:
       case LoopHintAttr::Interleave:
         setVectorizeEnable(true);
         break;
       case LoopHintAttr::Unroll:
-        setUnrollEnable(true);
+        setUnrollState(LoopAttributes::Enable);
         break;
       case LoopHintAttr::UnrollCount:
       case LoopHintAttr::VectorizeWidth:
       case LoopHintAttr::InterleaveCount:
         llvm_unreachable("Options cannot enabled.");
         break;
       }
       break;
     case LoopHintAttr::AssumeSafety:
       switch (Option) {
       case LoopHintAttr::Vectorize:
       case LoopHintAttr::Interleave:
         // Apply "llvm.mem.parallel_loop_access" metadata to load/stores.
         setParallel(true);
         setVectorizeEnable(true);
         break;
       case LoopHintAttr::Unroll:
       case LoopHintAttr::UnrollCount:
       case LoopHintAttr::VectorizeWidth:
       case LoopHintAttr::InterleaveCount:
         llvm_unreachable("Options cannot be used to assume mem safety.");
         break;
       }
       break;
-    case LoopHintAttr::Default:
+    case LoopHintAttr::Full:
+      switch (Option) {
+      case LoopHintAttr::Unroll:
+        setUnrollState(LoopAttributes::Full);
+        break;
+      case LoopHintAttr::Vectorize:
+      case LoopHintAttr::Interleave:
+      case LoopHintAttr::UnrollCount:
+      case LoopHintAttr::VectorizeWidth:
+      case LoopHintAttr::InterleaveCount:
+        llvm_unreachable("Options cannot be used with 'full' hint.");
+        break;
+      }
+      break;
+    case LoopHintAttr::Numeric:
       switch (Option) {
       case LoopHintAttr::VectorizeWidth:
         setVectorizeWidth(ValueInt);
         break;
       case LoopHintAttr::InterleaveCount:
         setInterleaveCount(ValueInt);
         break;
       case LoopHintAttr::UnrollCount:
         setUnrollCount(ValueInt);
         break;
       case LoopHintAttr::Unroll:
-        // The default option is used when '#pragma unroll' is specified.
-        setUnrollEnable(true);
-        break;
       case LoopHintAttr::Vectorize:
       case LoopHintAttr::Interleave:
-        llvm_unreachable("Options cannot be assigned a value and do not have a "
-                         "default value.");
+        llvm_unreachable("Options cannot be assigned a value.");
         break;
       }
       break;
     }
   }
 
   /// Stage the attributes.
   push(Header);

lib/Parse/ParsePragma.cpp

bool Parser::HandlePragmaLoopHint(LoopHint &Hint) {
   }
 
   // Validate the argument.
   if (StateOption) {
     ConsumeToken(); // The annotation token.
     SourceLocation StateLoc = Toks[0].getLocation();
     IdentifierInfo *StateInfo = Toks[0].getIdentifierInfo();
     if (!StateInfo ||
-        ((OptionUnroll ? !StateInfo->isStr("full")
-                       : !StateInfo->isStr("enable") &&
-                             !StateInfo->isStr("assume_safety")) &&
-         !StateInfo->isStr("disable"))) {
+        (!StateInfo->isStr("enable") && !StateInfo->isStr("disable") &&
+         (OptionUnroll && !StateInfo->isStr("full") ||
+          !OptionUnroll && !StateInfo->isStr("assume_safety")))) {
       Diag(Toks[0].getLocation(), diag::err_pragma_invalid_keyword)
           << /*FullKeyword=*/OptionUnroll;
       return false;
     }
     if (TokSize > 2)
       Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
           << PragmaLoopHintString(Info->PragmaName, Info->Option);
     Hint.StateLoc = IdentifierLoc::create(Actions.Context, StateLoc, StateInfo);

 ///    'unroll_count' '(' loop-hint-value ')'
 ///
 ///  loop-hint-keyword:
 ///    'enable'
 ///    'disable'
 ///    'assume_safety'
 ///
 ///  unroll-hint-keyword:
-///    'full'
+///    'enable'
 ///    'disable'
+///    'full'
 ///
 ///  loop-hint-value:
 ///    constant-expression
 ///
 /// Specifying vectorize(enable) or vectorize_width(_value_) instructs llvm to
 /// try vectorizing the instructions of the loop it precedes. Specifying
 /// interleave(enable) or interleave_count(_value_) instructs llvm to try
 /// interleaving multiple iterations of the loop it precedes. The width of the
 /// vector instructions is specified by vectorize_width() and the number of
 /// interleaved loop iterations is specified by interleave_count(). Specifying a
 /// value of 1 effectively disables vectorization/interleaving, even if it is
 /// possible and profitable, and 0 is invalid. The loop vectorizer currently
 /// only works on inner loops.
 ///
 /// The unroll and unroll_count directives control the concatenation
-/// unroller. Specifying unroll(full) instructs llvm to try to
-/// unroll the loop completely, and unroll(disable) disables unrolling
-/// for the loop. Specifying unroll_count(_value_) instructs llvm to
-/// try to unroll the loop the number of times indicated by the value.
+/// unroller. Specifying unroll(enable) instructs llvm to unroll the loop
+/// completely if the trip count is known at compile time and unroll partially
+/// if the trip count is not known.  Specifying unroll(full) is similar to
+/// unroll(enable) but will unroll the loop only if the trip count is known at
+/// compile time.  Specifying unroll(disable) disables unrolling for the
+/// loop. Specifying unroll_count(_value_) instructs llvm to try to unroll the
+/// loop the number of times indicated by the value.
 void PragmaLoopHintHandler::HandlePragma(Preprocessor &PP,
                                          PragmaIntroducerKind Introducer,
                                          Token &Tok) {
   // Incoming token is "loop" from "#pragma clang loop".
   Token PragmaName = Tok;
   SmallVector<Token, 1> TokenList;
 
   // Lex the optimization option and verify it is an identifier.

lib/Sema/SemaStmtAttr.cpp

const char *Pragma =
         llvm::StringSwitch<const char *>(PragmaNameLoc->Ident->getName())
             .Case("unroll", "#pragma unroll")
             .Case("nounroll", "#pragma nounroll")
             .Default("#pragma clang loop");
     S.Diag(St->getLocStart(), diag::err_pragma_loop_precedes_nonloop) << Pragma;
     return nullptr;
   }
 
-  LoopHintAttr::OptionType Option;
   LoopHintAttr::Spelling Spelling;
-  if (PragmaUnroll) {
-    Option = ValueExpr ? LoopHintAttr::UnrollCount : LoopHintAttr::Unroll;
+  LoopHintAttr::OptionType Option;
+  LoopHintAttr::LoopHintState State;
+  if (PragmaNoUnroll) {
+    // #pragma nounroll
+    Spelling = LoopHintAttr::Pragma_nounroll;
+    Option = LoopHintAttr::Unroll;
+    State = LoopHintAttr::Disable;
+  } else if (PragmaUnroll) {
     Spelling = LoopHintAttr::Pragma_unroll;
-  } else if (PragmaNoUnroll) {
+    if (ValueExpr) {
+      // #pragma unroll N
+      Option = LoopHintAttr::UnrollCount;
+      State = LoopHintAttr::Numeric;
+    } else {
+      // #pragma unroll
       Option = LoopHintAttr::Unroll;
-    Spelling = LoopHintAttr::Pragma_nounroll;
+      State = LoopHintAttr::Enable;
+    }
   } else {
+    // #pragma clang loop ...
+    Spelling = LoopHintAttr::Pragma_clang_loop;
     assert(OptionLoc && OptionLoc->Ident &&
            "Attribute must have valid option info.");
-    IdentifierInfo *OptionInfo = OptionLoc->Ident;
-    Option = llvm::StringSwitch<LoopHintAttr::OptionType>(OptionInfo->getName())
+    Option = llvm::StringSwitch<LoopHintAttr::OptionType>(
+                 OptionLoc->Ident->getName())
                  .Case("vectorize", LoopHintAttr::Vectorize)
                  .Case("vectorize_width", LoopHintAttr::VectorizeWidth)
                  .Case("interleave", LoopHintAttr::Interleave)
                  .Case("interleave_count", LoopHintAttr::InterleaveCount)
                  .Case("unroll", LoopHintAttr::Unroll)
                  .Case("unroll_count", LoopHintAttr::UnrollCount)
                  .Default(LoopHintAttr::Vectorize);
-    Spelling = LoopHintAttr::Pragma_clang_loop;
-  }
-
-  LoopHintAttr::LoopHintState State = LoopHintAttr::Default;
-  if (PragmaNoUnroll) {
-    State = LoopHintAttr::Disable;
-  } else if (Option == LoopHintAttr::VectorizeWidth ||
-             Option == LoopHintAttr::InterleaveCount ||
-             Option == LoopHintAttr::UnrollCount) {
+    if (Option == LoopHintAttr::VectorizeWidth ||
       assert(ValueExpr && "Attribute must have a valid value expression.");
       if (S.CheckLoopHintExpr(ValueExpr, St->getLocStart()))
         return nullptr;
+      State = LoopHintAttr::Numeric;
     } else if (Option == LoopHintAttr::Vectorize ||
                Option == LoopHintAttr::Interleave ||
                Option == LoopHintAttr::Unroll) {
-    // Default state is assumed if StateLoc is not specified, such as with
-    // '#pragma unroll'.
-    if (StateLoc && StateLoc->Ident) {
+      assert(StateLoc && StateLoc->Ident && "Loop hint must have an argument");
       if (StateLoc->Ident->isStr("disable"))
         State = LoopHintAttr::Disable;
       else if (StateLoc->Ident->isStr("assume_safety"))
         State = LoopHintAttr::AssumeSafety;
-      else
+      else if (StateLoc->Ident->isStr("full"))
+        State = LoopHintAttr::Full;
+      else if (StateLoc->Ident->isStr("enable"))
         State = LoopHintAttr::Enable;
-    }
+      else
+        llvm_unreachable("bad loop hint argument");
+    } else
+      llvm_unreachable("bad loop hint");
   }
 
   return LoopHintAttr::CreateImplicit(S.Context, Spelling, Option, State,
                                       ValueExpr, A.getRange());
 }
 
 static void
 CheckForIncompatibleAttributes(Sema &S,
if (PrevAttr)
           << /*Duplicate=*/true << PrevAttr->getDiagnosticName(Policy)
           << LH->getDiagnosticName(Policy);
 
     if (CategoryState.StateAttr && CategoryState.NumericAttr &&
         (Category == Unroll ||
          CategoryState.StateAttr->getState() == LoopHintAttr::Disable)) {
       // Disable hints are not compatible with numeric hints of the same
       // category.  As a special case, numeric unroll hints are also not
-      // compatible with "enable" form of the unroll pragma, unroll(full).
+      // compatible with enable or full form of the unroll pragma because these
+      // directives indicate full unrolling.
       S.Diag(OptionLoc, diag::err_pragma_loop_compatibility)
           << /*Duplicate=*/false
           << CategoryState.StateAttr->getDiagnosticName(Policy)
           << CategoryState.NumericAttr->getDiagnosticName(Policy);
     }
   }
 }
 

test/CodeGenCXX/pragma-unroll.cpp

 // Use templates defined above. Test verifies metadata is generated correctly.
 void template_test(double *List, int Length) {
   double Value = 10;
 
   for_template_test<double>(List, Length, Value);
   for_template_define_test<double>(List, Length, Value);
 }
 
-// CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[UNROLL_FULL:.*]]}
-// CHECK: ![[UNROLL_FULL]] = !{!"llvm.loop.unroll.full"}
+// CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[UNROLL_ENABLE:.*]]}
+// CHECK: ![[UNROLL_ENABLE]] = !{!"llvm.loop.unroll.enable"}
 // CHECK: ![[LOOP_2]] = distinct !{![[LOOP_2:.*]], ![[UNROLL_DISABLE:.*]]}
 // CHECK: ![[UNROLL_DISABLE]] = !{!"llvm.loop.unroll.disable"}
 // CHECK: ![[LOOP_3]] = distinct !{![[LOOP_3]], ![[UNROLL_8:.*]]}
 // CHECK: ![[UNROLL_8]] = !{!"llvm.loop.unroll.count", i32 8}
 // CHECK: ![[LOOP_4]] = distinct !{![[LOOP_4]], ![[UNROLL_4:.*]]}
 // CHECK: ![[UNROLL_4]] = !{!"llvm.loop.unroll.count", i32 4}
 // CHECK: ![[LOOP_5]] = distinct !{![[LOOP_5]], ![[UNROLL_8:.*]]}
 // CHECK: ![[LOOP_6]] = distinct !{![[LOOP_6]], ![[UNROLL_8:.*]]}
 // CHECK: ![[LOOP_7]] = distinct !{![[LOOP_7]], ![[UNROLL_8:.*]]}

test/Parser/pragma-loop-safety.cpp

 #pragma clang loop interleave(assume_safety)
   while (i + 1 < Length) {
     List[i] = i;
   }
 
 /* expected-error {{expected ')'}} */ #pragma clang loop vectorize(assume_safety
 /* expected-error {{expected ')'}} */ #pragma clang loop interleave(assume_safety
 
-/* expected-error {{invalid argument; expected 'full' or 'disable'}} */ #pragma clang loop unroll(assume_safety)
+/* expected-error {{invalid argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll(assume_safety)
 
 /* expected-error {{invalid argument; expected 'enable', 'assume_safety' or 'disable'}} */ #pragma clang loop vectorize(badidentifier)
 /* expected-error {{invalid argument; expected 'enable', 'assume_safety' or 'disable'}} */ #pragma clang loop interleave(badidentifier)
-/* expected-error {{invalid argument; expected 'full' or 'disable'}} */ #pragma clang loop unroll(badidentifier)
+/* expected-error {{invalid argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll(badidentifier)
   while (i-7 < Length) {
     List[i] = i;
   }
 
 /* expected-error {{duplicate directives 'vectorize(assume_safety)' and 'vectorize(enable)'}} */ #pragma clang loop vectorize(enable)
 #pragma clang loop vectorize(assume_safety)
 /* expected-error {{duplicate directives 'interleave(assume_safety)' and 'interleave(enable)'}} */ #pragma clang loop interleave(enable)
 #pragma clang loop interleave(assume_safety)
   while (i-9 < Length) {
     List[i] = i;
   }
 }

test/Parser/pragma-loop.cpp

 /* expected-error {{expected ')'}} */ #pragma clang loop unroll(full
 
 /* expected-error {{expected ')'}} */ #pragma clang loop vectorize_width(4
 /* expected-error {{expected ')'}} */ #pragma clang loop interleave_count(4
 /* expected-error {{expected ')'}} */ #pragma clang loop unroll_count(4
 
 /* expected-error {{missing argument; expected 'enable', 'assume_safety' or 'disable'}} */ #pragma clang loop vectorize()
 /* expected-error {{missing argument; expected an integer value}} */ #pragma clang loop interleave_count()
-/* expected-error {{missing argument; expected 'full' or 'disable'}} */ #pragma clang loop unroll()
+/* expected-error {{missing argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll()
 
 /* expected-error {{missing option; expected vectorize, vectorize_width, interleave, interleave_count, unroll, or unroll_count}} */ #pragma clang loop
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword(enable)
 /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop vectorize(enable) badkeyword(4)
 /* expected-warning {{extra tokens at end of '#pragma clang loop'}} */ #pragma clang loop vectorize(enable) ,
   while (i-4 < Length) {
     List[i] = i;

 /* expected-error {{use of undeclared identifier 'badvalue'}} */ #pragma clang loop interleave_count(badvalue)
 /* expected-error {{use of undeclared identifier 'badvalue'}} */ #pragma clang loop unroll_count(badvalue)
   while (i-6 < Length) {
     List[i] = i;
   }
 
 /* expected-error {{invalid argument; expected 'enable', 'assume_safety' or 'disable'}} */ #pragma clang loop vectorize(badidentifier)
 /* expected-error {{invalid argument; expected 'enable', 'assume_safety' or 'disable'}} */ #pragma clang loop interleave(badidentifier)
-/* expected-error {{invalid argument; expected 'full' or 'disable'}} */ #pragma clang loop unroll(badidentifier)
+/* expected-error {{invalid argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll(badidentifier)
   while (i-7 < Length) {
     List[i] = i;
   }
 
 // PR20069 - Loop pragma arguments that are not identifiers or numeric
 // constants crash FE.
 /* expected-error {{expected ')'}} */ #pragma clang loop vectorize(()
 /* expected-error {{invalid argument; expected 'enable', 'assume_safety' or 'disable'}} */ #pragma clang loop interleave(*)
-/* expected-error {{invalid argument; expected 'full' or 'disable'}} */ #pragma clang loop unroll(=)
+/* expected-error {{invalid argument; expected 'enable', 'full' or 'disable'}} */ #pragma clang loop unroll(=)
 /* expected-error {{type name requires a specifier or qualifier}} expected-error {{expected expression}} */ #pragma clang loop vectorize_width(^)
 /* expected-error {{expected expression}} expected-error {{expected expression}} */ #pragma clang loop interleave_count(/)
 /* expected-error {{expected expression}} expected-error {{expected expression}} */ #pragma clang loop unroll_count(==)
   while (i-8 < Length) {
     List[i] = i;
   }
 
 #pragma clang loop vectorize(enable)

test/Parser/pragma-unroll.cpp

#pragma clang loop unroll(disable)
   }
 
 /* expected-error {{incompatible directives 'unroll(full)' and '#pragma unroll(4)'}} */ #pragma unroll(4)
 #pragma clang loop unroll(full)
   while (i-11 < Length) {
     List[i] = i;
   }
 
+/* expected-error {{incompatible directives 'unroll(enable)' and '#pragma unroll(4)'}} */ #pragma unroll(4)
+#pragma clang loop unroll(enable)
+  while (i-11 < Length) {
+    List[i] = i;
+  }
+
 /* expected-error {{incompatible directives '#pragma unroll' and '#pragma unroll(4)'}} */ #pragma unroll(4)
 #pragma unroll
   while (i-11 < Length) {
     List[i] = i;
   }
 
 /* expected-error {{incompatible directives '#pragma nounroll' and 'unroll_count(4)'}} */ #pragma clang loop unroll_count(4)
 #pragma nounroll