1 files changed, 58 insertions, 46 deletions

diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index 4cb0fde11d0..595aa3656eb 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -47,11 +47,13 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const AttributeList &A,
                                 SourceRange) {
   IdentifierLoc *PragmaNameLoc = A.getArgAsIdent(0);
   IdentifierLoc *OptionLoc = A.getArgAsIdent(1);
-  IdentifierLoc *StateLoc = A.getArgAsIdent(2);
+  IdentifierInfo *OptionInfo = OptionLoc->Ident;
+  IdentifierLoc *ValueLoc = A.getArgAsIdent(2);
+  IdentifierInfo *ValueInfo = ValueLoc ? ValueLoc->Ident : nullptr;
   Expr *ValueExpr = A.getArgAsExpr(3);
 
-  bool PragmaUnroll = PragmaNameLoc->Ident->getName() == "unroll";
-  bool PragmaNoUnroll = PragmaNameLoc->Ident->getName() == "nounroll";
+  assert(OptionInfo && "Attribute must have valid option info.");
+
   if (St->getStmtClass() != Stmt::DoStmtClass &&
       St->getStmtClass() != Stmt::ForStmtClass &&
       St->getStmtClass() != Stmt::CXXForRangeStmtClass &&
@@ -67,16 +69,13 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const AttributeList &A,
 
   LoopHintAttr::OptionType Option;
   LoopHintAttr::Spelling Spelling;
-  if (PragmaUnroll) {
-    Option = ValueExpr ? LoopHintAttr::UnrollCount : LoopHintAttr::Unroll;
+  if (PragmaNameLoc->Ident->getName() == "unroll") {
+    Option = ValueLoc ? LoopHintAttr::UnrollCount : LoopHintAttr::Unroll;
     Spelling = LoopHintAttr::Pragma_unroll;
-  } else if (PragmaNoUnroll) {
+  } else if (PragmaNameLoc->Ident->getName() == "nounroll") {
     Option = LoopHintAttr::Unroll;
     Spelling = LoopHintAttr::Pragma_nounroll;
   } else {
-    assert(OptionLoc && OptionLoc->Ident &&
-           "Attribute must have valid option info.");
-    IdentifierInfo *OptionInfo = OptionLoc->Ident;
     Option = llvm::StringSwitch<LoopHintAttr::OptionType>(OptionInfo->getName())
                  .Case("vectorize", LoopHintAttr::Vectorize)
                  .Case("vectorize_width", LoopHintAttr::VectorizeWidth)
@@ -88,10 +87,35 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const AttributeList &A,
     Spelling = LoopHintAttr::Pragma_clang_loop;
   }
 
-  int ValueInt = 1;
-  LoopHintAttr::LoopHintState State = LoopHintAttr::Default;
-  if (PragmaNoUnroll) {
-    State = LoopHintAttr::Disable;
+  int ValueInt = -1;
+  if (Option == LoopHintAttr::Unroll &&
+      Spelling == LoopHintAttr::Pragma_unroll) {
+    if (ValueInfo)
+      ValueInt = (ValueInfo->isStr("disable") ? 0 : 1);
+  } else if (Option == LoopHintAttr::Unroll &&
+             Spelling == LoopHintAttr::Pragma_nounroll) {
+    ValueInt = 0;
+  } else if (Option == LoopHintAttr::Vectorize ||
+             Option == LoopHintAttr::Interleave ||
+             Option == LoopHintAttr::Unroll) {
+    // Unrolling uses the keyword "full" rather than "enable" to indicate full
+    // unrolling.
+    const char *TrueKeyword =
+        Option == LoopHintAttr::Unroll ? "full" : "enable";
+    if (!ValueInfo) {
+      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword)
+          << TrueKeyword;
+      return nullptr;
+    }
+    if (ValueInfo->isStr("disable"))
+      ValueInt = 0;
+    else if (ValueInfo->getName() == TrueKeyword)
+      ValueInt = 1;
+    else {
+      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword)
+          << TrueKeyword;
+      return nullptr;
+    }
   } else if (Option == LoopHintAttr::VectorizeWidth ||
              Option == LoopHintAttr::InterleaveCount ||
              Option == LoopHintAttr::UnrollCount) {
@@ -100,39 +124,28 @@ static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const AttributeList &A,
     llvm::APSInt ValueAPS;
     if (!ValueExpr || !ValueExpr->isIntegerConstantExpr(ValueAPS, S.Context) ||
         (ValueInt = ValueAPS.getSExtValue()) < 1) {
-      S.Diag(A.getLoc(), diag::err_pragma_loop_invalid_value);
+      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_value);
       return nullptr;
     }
-  } 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) {
-      if (StateLoc->Ident->isStr("disable"))
-        State = LoopHintAttr::Disable;
-      else
-        State = LoopHintAttr::Enable;
-    }
-  }
+  } else
+    llvm_unreachable("Unknown loop hint option");
 
-  return LoopHintAttr::CreateImplicit(S.Context, Spelling, Option, State,
-                                      ValueInt, A.getRange());
+  return LoopHintAttr::CreateImplicit(S.Context, Spelling, Option, ValueInt,
+                                      A.getRange());
 }
 
-static void
-CheckForIncompatibleAttributes(Sema &S,
-                               const SmallVectorImpl<const Attr *> &Attrs) {
-  // There are 3 categories of loop hints attributes: vectorize, interleave,
-  // and unroll. Each comes in two variants: a state form and a numeric form.
-  // The state form selectively defaults/enables/disables the transformation
-  // for the loop (for unroll, default indicates full unrolling rather than
-  // enabling the transformation).  The numeric form form provides an integer
-  // hint (for example, unroll count) to the transformer. The following array
-  // accumulates the hints encountered while iterating through the attributes
-  // to check for compatibility.
+static void CheckForIncompatibleAttributes(
+    Sema &S, const SmallVectorImpl<const Attr *> &Attrs) {
+  // There are 3 categories of loop hints attributes: vectorize, interleave, and
+  // unroll. Each comes in two variants: a boolean form and a numeric form.  The
+  // boolean hints selectively enables/disables the transformation for the loop
+  // (for unroll, a nonzero value indicates full unrolling rather than enabling
+  // the transformation).  The numeric hint provides an integer hint (for
+  // example, unroll count) to the transformer. The following array accumulates
+  // the hints encountered while iterating through the attributes to check for
+  // compatibility.
   struct {
-    const LoopHintAttr *StateAttr;
+    const LoopHintAttr *EnableAttr;
     const LoopHintAttr *NumericAttr;
   } HintAttrs[] = {{nullptr, nullptr}, {nullptr, nullptr}, {nullptr, nullptr}};
 
@@ -166,8 +179,8 @@ CheckForIncompatibleAttributes(Sema &S,
     if (Option == LoopHintAttr::Vectorize ||
         Option == LoopHintAttr::Interleave || Option == LoopHintAttr::Unroll) {
       // Enable|disable hint.  For example, vectorize(enable).
-      PrevAttr = CategoryState.StateAttr;
-      CategoryState.StateAttr = LH;
+      PrevAttr = CategoryState.EnableAttr;
+      CategoryState.EnableAttr = LH;
     } else {
       // Numeric hint.  For example, vectorize_width(8).
       PrevAttr = CategoryState.NumericAttr;
@@ -182,15 +195,14 @@ CheckForIncompatibleAttributes(Sema &S,
           << /*Duplicate=*/true << PrevAttr->getDiagnosticName(Policy)
           << LH->getDiagnosticName(Policy);
 
-    if (CategoryState.StateAttr && CategoryState.NumericAttr &&
-        (Category == Unroll ||
-         CategoryState.StateAttr->getState() == LoopHintAttr::Disable)) {
+    if (CategoryState.EnableAttr && CategoryState.NumericAttr &&
+        (Category == Unroll || !CategoryState.EnableAttr->getValue())) {
       // 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).
       S.Diag(OptionLoc, diag::err_pragma_loop_compatibility)
           << /*Duplicate=*/false
-          << CategoryState.StateAttr->getDiagnosticName(Policy)
+          << CategoryState.EnableAttr->getDiagnosticName(Policy)
           << CategoryState.NumericAttr->getDiagnosticName(Policy);
     }
   }