Change unroll metadata and unroll pragma names

Aaron Ballman aaron.ballman at gmail.com
Tue Jul 22 12:06:57 PDT 2014


On Tue, Jul 22, 2014 at 2:33 PM, Mark Heffernan <meheff at google.com> wrote:
> This patch does some renaming of the metadata and unrolling pragma terms to
> hopefully remove a source of confusion.  It was motivated the comments in
> review: http://reviews.llvm.org/D4571.  Prior to this patch we have the
> following pragma syntax and the corresponding metadata it generates:
>
> #pragma clang loop unroll(enable)  => !{metadata !"llvm.loop.unroll.enable",
> i1 true}
> #pragma clang loop unroll(disable)  => !{metadata
> !"llvm.loop.unroll.enable", i1 false}
> #pragma clang loop unroll_count(N) => !{metadata !"llvm.loop.unroll.count",
> i32 N}
>
> #pragma unroll => !{metadata !"llvm.loop.unroll.enable", i1 true}
> #pragma unroll N => !{metadata !"llvm.loop.unroll.count", i32 N}
>
> Unroll disable and unroll count are pretty self explanatory.  The issue is
> with "unroll(enable)".  "unroll(enable)" indicates to unroll the loop fully
> which is a bit surprising.  It also is not clear whether you need
> "unroll(enable)" when you specify "unroll_count(N)".  Both of these
> potential sources of confusion are resolved with this patch which changes
> the pragma and metadata to:
>
> #pragma clang loop unroll(full)  => !{metadata !"llvm.loop.unroll.full"}
> #pragma clang loop unroll(disable)  => !{metadata
> !"llvm.loop.unroll.disable"}
> #pragma clang loop unroll_count(N) => !{metadata !"llvm.loop.unroll.count",
> i32 N}
>
> #pragma unroll => !{metadata !"llvm.loop.unroll.full"}
> #pragma unroll N => !{metadata !"llvm.loop.unroll.count", i32 N}
>
> Now it should (hopefully) be immediately clear what each of these do.  The
> loop unrolling transformation itself is not affected.  Other than renaming,
> the only functional difference is that a loop can now only have a single
> loop unroll directive.  Previously (and confusingly) a loop could have both
> unroll(enable) and unroll_count(N).

Some very minor comments below on the clang side of the patch. Aside
from these nits, LGTM.

> Index: docs/LanguageExtensions.rst
> ===================================================================
> --- docs/LanguageExtensions.rst (revision 213580)
> +++ docs/LanguageExtensions.rst (working copy)
> @@ -1830,7 +1830,7 @@
>  compile time. Partial unrolling replicates the loop body within the loop and
>  reduces the trip count.
>
> -If ``unroll(enable)`` is specified the unroller will attempt to fully unroll the
> +If ``unroll(full)`` is specified the unroller will attempt to fully unroll the
>  loop if the trip count is known at compile time. If the loop count is not known
>  or the fully unrolled code size is greater than the limit specified by the
>  `-pragma-unroll-threshold` command line option the loop will be partially
> @@ -1838,7 +1838,7 @@
>
>  .. code-block:: c++
>
> -  #pragma clang loop unroll(enable)
> +  #pragma clang loop unroll(full)
>    for(...) {
>      ...
>    }
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td (revision 213580)
> +++ include/clang/Basic/Attr.td (working copy)
> @@ -1776,7 +1776,7 @@
>    /// vectorize_width: vectorize loop operations with width 'value'.
>    /// interleave: interleave multiple loop iterations if 'value != 0'.
>    /// interleave_count: interleaves 'value' loop interations.
> -  /// unroll: unroll loop if 'value != 0'.
> +  /// unroll: fully unroll loop if 'value != 0'.
>    /// unroll_count: unrolls loop 'value' times.
>
>    let Spellings = [Pragma<"clang", "loop">, Pragma<"", "unroll">];
> @@ -1830,6 +1830,10 @@
>      if (option == VectorizeWidth || option == InterleaveCount ||
>          option == UnrollCount)
>        OS << value;
> +    else if (option == Unroll && value)
> +      // Unroll loop hint does not use the keyword "enable". Instead, a true value
> +      // indicate full unrolling which uses the keyword "full".

Typo: indicates? Also, instead of "true value", perhaps a "nonzero
value" would read a bit easier, since these are integer arguments.

> +      OS << "full";
>      else if (value)
>        OS << "enable";
>      else
> Index: include/clang/Basic/AttrDocs.td
> ===================================================================
> --- include/clang/Basic/AttrDocs.td (revision 213580)
> +++ include/clang/Basic/AttrDocs.td (working copy)
> @@ -1080,7 +1080,7 @@
>    }
>
>  ``#pragma unroll`` and ``#pragma unroll _value_`` have identical semantics to
> -``#pragma clang loop unroll(enable)`` and ``#pragma clang loop
> +``#pragma clang loop unroll(full)`` and ``#pragma clang loop
>  unroll_count(_value_)`` respectively. See `language extensions
>  <http://clang.llvm.org/docs/LanguageExtensions.html#extensions-for-loop-hint-optimizations>`_
>  for further details including limitations of the unroll hints.
> Index: include/clang/Basic/DiagnosticParseKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticParseKinds.td (revision 213580)
> +++ include/clang/Basic/DiagnosticParseKinds.td (working copy)
> @@ -815,7 +815,7 @@
>    "expected non-wide string literal in '#pragma %0'">, InGroup<IgnoredPragmas>;
>  // - Generic errors
>  def err_pragma_missing_argument : Error<
> -  "missing argument to '#pragma %0'; expected %1">;
> +  "missing argument to '#pragma %0'%select{|; expected %2}1">;
>  // - #pragma options
>  def warn_pragma_options_expected_align : Warning<
>    "expected 'align' following '#pragma options' - ignored">,
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td (revision 213580)
> +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
> @@ -539,7 +539,7 @@
>  def err_pragma_loop_invalid_value : Error<
>    "invalid argument; expected a positive integer value">;
>  def err_pragma_loop_invalid_keyword : Error<
> -  "invalid argument; expected 'enable' or 'disable'">;
> +  "invalid argument; expected '%0' or 'disable'">;
>  def err_pragma_loop_compatibility : Error<
>    "%select{incompatible|duplicate}0 directives '%1' and '%2'">;
>  def err_pragma_loop_precedes_nonloop : Error<
> Index: lib/CodeGen/CGStmt.cpp
> ===================================================================
> --- lib/CodeGen/CGStmt.cpp (revision 213580)
> +++ lib/CodeGen/CGStmt.cpp (working copy)
> @@ -586,55 +586,53 @@
>
>      const char *MetadataName;
>      switch (Option) {
> -    case LoopHintAttr::Vectorize:
> -    case LoopHintAttr::VectorizeWidth:
> -      MetadataName = "llvm.loop.vectorize.width";
> -      break;
> -    case LoopHintAttr::Interleave:
> -    case LoopHintAttr::InterleaveCount:
> -      MetadataName = "llvm.loop.vectorize.unroll";
> -      break;
> -    case LoopHintAttr::Unroll:
> -      MetadataName = "llvm.loop.unroll.enable";
> -      break;
> -    case LoopHintAttr::UnrollCount:
> -      MetadataName = "llvm.loop.unroll.count";
> -      break;
> +      case LoopHintAttr::Vectorize:
> +      case LoopHintAttr::VectorizeWidth:
> +        MetadataName = "llvm.loop.vectorize.width";
> +        break;
> +      case LoopHintAttr::Interleave:
> +      case LoopHintAttr::InterleaveCount:
> +        MetadataName = "llvm.loop.vectorize.unroll";
> +        break;
> +      case LoopHintAttr::Unroll:
> +        // With the unroll loop hint, a non-zero value indicates full unrolling.
> +        MetadataName = ValueInt == 0 ? "llvm.loop.unroll.disable"
> +                                     : "llvm.loop.unroll.full";
> +        break;
> +      case LoopHintAttr::UnrollCount:
> +        MetadataName = "llvm.loop.unroll.count";
> +        break;

The formatting here does not match our style guides.

>      }
> -
>      llvm::Value *Value;
>      llvm::MDString *Name;
>      switch (Option) {
> -    case LoopHintAttr::Vectorize:
> -    case LoopHintAttr::Interleave:
> -      if (ValueInt == 1) {
> -        // FIXME: In the future I will modifiy the behavior of the metadata
> -        // so we can enable/disable vectorization and interleaving separately.
> -        Name = llvm::MDString::get(Context, "llvm.loop.vectorize.enable");
> -        Value = Builder.getTrue();
> +      case LoopHintAttr::Vectorize:
> +      case LoopHintAttr::Interleave:
> +        if (ValueInt == 1) {
> +          // FIXME: In the future I will modifiy the behavior of the metadata
> +          // so we can enable/disable vectorization and interleaving separately.
> +          Name = llvm::MDString::get(Context, "llvm.loop.vectorize.enable");
> +          Value = Builder.getTrue();
> +          break;
> +        }
> +        // Vectorization/interleaving is disabled, set width/count to 1.
> +        ValueInt = 1;
> +      // Fallthrough.
> +      case LoopHintAttr::VectorizeWidth:
> +      case LoopHintAttr::InterleaveCount:
> +      case LoopHintAttr::UnrollCount:
> +        Name = llvm::MDString::get(Context, MetadataName);
> +        Value = llvm::ConstantInt::get(Int32Ty, ValueInt);
>          break;
> -      }
> -      // Vectorization/interleaving is disabled, set width/count to 1.
> -      ValueInt = 1;
> -      // Fallthrough.
> -    case LoopHintAttr::VectorizeWidth:
> -    case LoopHintAttr::InterleaveCount:
> -      Name = llvm::MDString::get(Context, MetadataName);
> -      Value = llvm::ConstantInt::get(Int32Ty, ValueInt);
> -      break;
> -    case LoopHintAttr::Unroll:
> -      Name = llvm::MDString::get(Context, MetadataName);
> -      Value = (ValueInt == 0) ? Builder.getFalse() : Builder.getTrue();
> -      break;
> -    case LoopHintAttr::UnrollCount:
> -      Name = llvm::MDString::get(Context, MetadataName);
> -      Value = llvm::ConstantInt::get(Int32Ty, ValueInt);
> -      break;
> +      case LoopHintAttr::Unroll:
> +        Name = llvm::MDString::get(Context, MetadataName);
> +        Value = nullptr;
> +        break;

Same issue with formatting here as above.

>      }
>
>      SmallVector<llvm::Value *, 2> OpValues;
>      OpValues.push_back(Name);
> -    OpValues.push_back(Value);
> +    if (Value) OpValues.push_back(Value);

The controlled statement should be on its own line.

>
>      // Set or overwrite metadata indicated by Name.
>      Metadata.push_back(llvm::MDNode::get(Context, OpValues));
> Index: lib/Parse/ParsePragma.cpp
> ===================================================================
> --- lib/Parse/ParsePragma.cpp (revision 213580)
> +++ lib/Parse/ParsePragma.cpp (working copy)
> @@ -1670,8 +1670,7 @@
>    PP.Lex(Tok);
>    if (Tok.is(tok::eod)) {
>      PP.Diag(Tok.getLocation(), diag::err_pragma_missing_argument)
> -        << "clang optimize"
> -        << "'on' or 'off'";
> +        << "clang optimize" << /*Expected=*/true << "'on' or 'off'";
>      return;
>    }
>    if (Tok.isNot(tok::identifier)) {
> @@ -1718,8 +1717,12 @@
>                 "Unexpected pragma name");
>          PragmaString = "unroll";
>        }
> +      // Don't try to emit what the pragma is expecting with the diagnostic
> +      // because the logic is non-trivial and we give expected values in sema
> +      // diagnostics if an invalid argument is given.  Here, just note that the
> +      // pragma is missing an argument.
>        PP.Diag(Tok.getLocation(), diag::err_pragma_missing_argument)
> -          << PragmaString << "a positive integer value";
> +          << PragmaString << /*Expected=*/false;
>        return true;
>      }
>    }
> @@ -1751,7 +1754,7 @@
>  ///  loop-hint:
>  ///    'vectorize' '(' loop-hint-keyword ')'
>  ///    'interleave' '(' loop-hint-keyword ')'
> -///    'unroll' '(' loop-hint-keyword ')'
> +///    'unroll' '(' unroll-hint-keyword ')'
>  ///    'vectorize_width' '(' loop-hint-value ')'
>  ///    'interleave_count' '(' loop-hint-value ')'
>  ///    'unroll_count' '(' loop-hint-value ')'
> @@ -1760,6 +1763,10 @@
>  ///    'enable'
>  ///    'disable'
>  ///
> +///  unroll-hint-keyword:
> +///    'full'
> +///    'disable'
> +///
>  ///  loop-hint-value:
>  ///    constant-expression
>  ///
> @@ -1774,12 +1781,10 @@
>  /// only works on inner loops.
>  ///
>  /// The unroll and unroll_count directives control the concatenation
> -/// unroller. Specifying unroll(enable) instructs llvm to try to
> +/// 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.
> -/// If unroll(enable) and unroll_count are both specified only
> -/// unroll_count takes effect.
>  void PragmaLoopHintHandler::HandlePragma(Preprocessor &PP,
>                                           PragmaIntroducerKind Introducer,
>                                           Token &Tok) {
> Index: lib/Sema/SemaStmtAttr.cpp
> ===================================================================
> --- lib/Sema/SemaStmtAttr.cpp (revision 213580)
> +++ lib/Sema/SemaStmtAttr.cpp (working copy)
> @@ -89,16 +89,22 @@
>    } 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);
> +      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword)
> +          << TrueKeyword;
>        return nullptr;
>      }
>      if (ValueInfo->isStr("disable"))
>        ValueInt = 0;
> -    else if (ValueInfo->isStr("enable"))
> +    else if (ValueInfo->getName() == TrueKeyword)
>        ValueInt = 1;
>      else {
> -      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword);
> +      S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword)
> +          << TrueKeyword;
>        return nullptr;
>      }
>    } else if (Option == LoopHintAttr::VectorizeWidth ||
> @@ -121,12 +127,14 @@
>
>  static void CheckForIncompatibleAttributes(
>      Sema &S, const SmallVectorImpl<const Attr *> &Attrs) {
> -  // There are 3 categories of loop hints: vectorize, interleave, and
> -  // unroll. Each comes in two variants: an enable/disable form and a
> -  // form which takes a numeric argument. For example:
> -  // unroll(enable|disable) and unroll_count(N). The following array
> -  // accumulate the hints encountered while iterating through the
> -  // attributes to check for compatibility.
> +  // 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 true value indicates full unrolling rather than enabling the

Instead of true value, perhaps "nonzero value"? Also, missing a comma
after "unroll."

> +  // transformation).  The numeric hint provides an integer hint (for example,
> +  // unroll count) to the transformer. The following array accumulate the hints

Pluralization mismatch (array accumulate).

> +  // encountered while iterating through the attributes to check for
> +  // compatibility.
>    struct {
>      const LoopHintAttr *EnableAttr;
>      const LoopHintAttr *NumericAttr;
> @@ -141,19 +149,20 @@
>
>      int Option = LH->getOption();
>      int Category;
> +    enum { Vectorize, Interleave, Unroll };
>      switch (Option) {
> -    case LoopHintAttr::Vectorize:
> -    case LoopHintAttr::VectorizeWidth:
> -      Category = 0;
> -      break;
> -    case LoopHintAttr::Interleave:
> -    case LoopHintAttr::InterleaveCount:
> -      Category = 1;
> -      break;
> -    case LoopHintAttr::Unroll:
> -    case LoopHintAttr::UnrollCount:
> -      Category = 2;
> -      break;
> +      case LoopHintAttr::Vectorize:
> +      case LoopHintAttr::VectorizeWidth:
> +        Category = Vectorize;
> +        break;
> +      case LoopHintAttr::Interleave:
> +      case LoopHintAttr::InterleaveCount:
> +        Category = Interleave;
> +        break;
> +      case LoopHintAttr::Unroll:
> +      case LoopHintAttr::UnrollCount:
> +        Category = Unroll;
> +        break;

Formatting.

>      };
>
>      auto &CategoryState = HintAttrs[Category];
> @@ -176,10 +185,11 @@
>            << /*Duplicate=*/true << PrevAttr->getDiagnosticName()
>            << LH->getDiagnosticName();
>
> -    if (CategoryState.EnableAttr && !CategoryState.EnableAttr->getValue() &&
> -        CategoryState.NumericAttr) {
> -      // Disable hints are not compatible with numeric hints of the
> -      // same category.
> +    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.EnableAttr->getDiagnosticName()
> Index: test/CodeGen/pragma-loop.cpp
> ===================================================================
> --- test/CodeGen/pragma-loop.cpp (revision 213580)
> +++ test/CodeGen/pragma-loop.cpp (working copy)
> @@ -8,7 +8,7 @@
>  #pragma clang loop vectorize(enable)
>  #pragma clang loop interleave_count(4)
>  #pragma clang loop vectorize_width(4)
> -#pragma clang loop unroll(enable)
> +#pragma clang loop unroll(full)
>    while (i < Length) {
>      // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_1:.*]]
>      List[i] = i * 2;
> @@ -108,13 +108,13 @@
>    for_template_define_test<double>(List, Length, Value);
>  }
>
> -// CHECK: ![[LOOP_1]] = metadata !{metadata ![[LOOP_1]], metadata ![[UNROLLENABLE_1:.*]], metadata ![[WIDTH_4:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[INTENABLE_1:.*]]}
> -// CHECK: ![[UNROLLENABLE_1]] = metadata !{metadata !"llvm.loop.unroll.enable", i1 true}
> +// CHECK: ![[LOOP_1]] = metadata !{metadata ![[LOOP_1]], metadata ![[UNROLL_FULL:.*]], metadata ![[WIDTH_4:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[INTENABLE_1:.*]]}
> +// CHECK: ![[UNROLL_FULL]] = metadata !{metadata !"llvm.loop.unroll.full"}
>  // CHECK: ![[WIDTH_4]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 4}
>  // CHECK: ![[INTERLEAVE_4]] = metadata !{metadata !"llvm.loop.vectorize.unroll", i32 4}
>  // CHECK: ![[INTENABLE_1]] = metadata !{metadata !"llvm.loop.vectorize.enable", i1 true}
> -// CHECK: ![[LOOP_2]] = metadata !{metadata ![[LOOP_2:.*]], metadata ![[UNROLLENABLE_0:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[WIDTH_8:.*]]}
> -// CHECK: ![[UNROLLENABLE_0]] = metadata !{metadata !"llvm.loop.unroll.enable", i1 false}
> +// CHECK: ![[LOOP_2]] = metadata !{metadata ![[LOOP_2:.*]], metadata ![[UNROLL_DISABLE:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[WIDTH_8:.*]]}
> +// CHECK: ![[UNROLL_DISABLE]] = metadata !{metadata !"llvm.loop.unroll.disable"}
>  // CHECK: ![[WIDTH_8]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 8}
>  // CHECK: ![[LOOP_3]] = metadata !{metadata ![[LOOP_3]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[ENABLE_1:.*]]}
>  // CHECK: ![[UNROLL_8]] = metadata !{metadata !"llvm.loop.unroll.count", i32 8}
> @@ -121,7 +121,7 @@
>  // CHECK: ![[LOOP_4]] = metadata !{metadata ![[LOOP_4]], metadata ![[INTERLEAVE_2:.*]], metadata ![[WIDTH_2:.*]]}
>  // CHECK: ![[INTERLEAVE_2]] = metadata !{metadata !"llvm.loop.vectorize.unroll", i32 2}
>  // CHECK: ![[WIDTH_2]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 2}
> -// CHECK: ![[LOOP_5]] = metadata !{metadata ![[LOOP_5]], metadata ![[UNROLLENABLE_0:.*]], metadata ![[WIDTH_1:.*]]}
> +// CHECK: ![[LOOP_5]] = metadata !{metadata ![[LOOP_5]], metadata ![[UNROLL_DISABLE:.*]], metadata ![[WIDTH_1:.*]]}
>  // CHECK: ![[WIDTH_1]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 1}
>  // CHECK: ![[LOOP_6]] = metadata !{metadata ![[LOOP_6]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_2:.*]], metadata ![[WIDTH_2:.*]]}
>  // CHECK: ![[LOOP_7]] = metadata !{metadata ![[LOOP_7]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_8:.*]], metadata ![[WIDTH_8:.*]]}
> Index: test/CodeGen/pragma-unroll.cpp
> ===================================================================
> --- test/CodeGen/pragma-unroll.cpp (revision 213580)
> +++ test/CodeGen/pragma-unroll.cpp (working copy)
> @@ -86,8 +86,8 @@
>    for_template_define_test<double>(List, Length, Value);
>  }
>
> -// CHECK: ![[LOOP_1]] = metadata !{metadata ![[LOOP_1]], metadata ![[UNROLLENABLE_1:.*]]}
> -// CHECK: ![[UNROLLENABLE_1]] = metadata !{metadata !"llvm.loop.unroll.enable", i1 true}
> +// CHECK: ![[LOOP_1]] = metadata !{metadata ![[LOOP_1]], metadata ![[UNROLL_FULL:.*]]}
> +// CHECK: ![[UNROLL_FULL]] = metadata !{metadata !"llvm.loop.unroll.full"}
>  // CHECK: ![[LOOP_2]] = metadata !{metadata ![[LOOP_2:.*]], metadata ![[UNROLL_16:.*]]}
>  // CHECK: ![[UNROLL_16]] = metadata !{metadata !"llvm.loop.unroll.count", i32 16}
>  // CHECK: ![[LOOP_3]] = metadata !{metadata ![[LOOP_3]], metadata ![[UNROLL_8:.*]]}
> Index: test/PCH/pragma-loop.cpp
> ===================================================================
> --- test/PCH/pragma-loop.cpp (revision 213580)
> +++ test/PCH/pragma-loop.cpp (working copy)
> @@ -10,7 +10,7 @@
>  // CHECK: #pragma clang loop unroll(disable)
>  // CHECK: #pragma clang loop interleave(disable)
>  // CHECK: #pragma clang loop vectorize(enable)
> -// CHECK: #pragma clang loop unroll(enable)
> +// CHECK: #pragma clang loop unroll(full)
>  // CHECK: #pragma clang loop interleave(enable)
>  // CHECK: #pragma clang loop vectorize(disable)
>  // CHECK: #pragma unroll
> @@ -47,7 +47,7 @@
>      int i = 0;
>  #pragma clang loop vectorize(disable)
>  #pragma clang loop interleave(enable)
> -#pragma clang loop unroll(enable)
> +#pragma clang loop unroll(full)
>      while (i - 3 < Length) {
>        List[i] = i;
>        i++;
> Index: test/Parser/pragma-loop.cpp
> ===================================================================
> --- test/Parser/pragma-loop.cpp (revision 213580)
> +++ test/Parser/pragma-loop.cpp (working copy)
> @@ -8,7 +8,7 @@
>
>  #pragma clang loop vectorize(enable)
>  #pragma clang loop interleave(enable)
> -#pragma clang loop unroll(enable)
> +#pragma clang loop unroll(full)
>    while (i + 1 < Length) {
>      List[i] = i;
>    }
> @@ -49,15 +49,15 @@
>
>  /* expected-error {{expected ')'}} */ #pragma clang loop vectorize(enable
>  /* expected-error {{expected ')'}} */ #pragma clang loop interleave(enable
> -/* expected-error {{expected ')'}} */ #pragma clang loop unroll(enable
> +/* 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 to '#pragma clang loop vectorize'; expected a positive integer value}} */ #pragma clang loop vectorize()
> -/* expected-error {{missing argument to '#pragma clang loop interleave_count'; expected a positive integer value}} */ #pragma clang loop interleave_count()
> -/* expected-error {{missing argument to '#pragma clang loop unroll'; expected a positive integer value}} */ #pragma clang loop unroll()
> +/* expected-error {{missing argument to '#pragma clang loop vectorize'}} */ #pragma clang loop vectorize()
> +/* expected-error {{missing argument to '#pragma clang loop interleave_count'}} */ #pragma clang loop interleave_count()
> +/* expected-error {{missing argument to '#pragma clang loop unroll'}} */ #pragma clang loop unroll()
>
>  /* expected-error {{missing option}} */ #pragma clang loop
>  /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword
> @@ -92,7 +92,7 @@
>
>  /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop vectorize(badidentifier)
>  /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop interleave(badidentifier)
> -/* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop unroll(badidentifier)
> +/* expected-error {{invalid argument; expected 'full' or 'disable'}} */ #pragma clang loop unroll(badidentifier)
>    while (i-7 < Length) {
>      List[i] = i;
>    }
> @@ -101,7 +101,7 @@
>  // constants crash FE.
>  /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop vectorize(()
>  /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop interleave(*)
> -/* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop unroll(=)
> +/* expected-error {{invalid argument; expected 'full' or 'disable'}} */ #pragma clang loop unroll(=)
>  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(^)
>  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(/)
>  /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(==)
> @@ -136,7 +136,7 @@
>  #pragma clang loop vectorize(disable)
>  /* expected-error {{duplicate directives 'interleave(disable)' and 'interleave(enable)'}} */ #pragma clang loop interleave(enable)
>  #pragma clang loop interleave(disable)
> -/* expected-error {{duplicate directives 'unroll(disable)' and 'unroll(enable)'}} */ #pragma clang loop unroll(enable)
> +/* expected-error {{duplicate directives 'unroll(disable)' and 'unroll(full)'}} */ #pragma clang loop unroll(full)
>  #pragma clang loop unroll(disable)
>    while (i-9 < Length) {
>      List[i] = i;
> @@ -162,5 +162,12 @@
>      List[i] = i;
>    }
>
> +
> +/* expected-error {{incompatible directives 'unroll(full)' and 'unroll_count(4)'}} */ #pragma clang loop unroll(full)
> +#pragma clang loop unroll_count(4)
> +  while (i-11 < Length) {
> +    List[i] = i;
> +  }
> +
>  #pragma clang loop interleave(enable)
>  /* expected-error {{expected statement}} */ }
> Index: test/Parser/pragma-unroll.cpp
> ===================================================================
> --- test/Parser/pragma-unroll.cpp (revision 213580)
> +++ test/Parser/pragma-unroll.cpp (working copy)
> @@ -21,26 +21,8 @@
>      List[i] = i;
>    }
>
> -#pragma unroll
> -#pragma unroll(8)
> -  while (i - 3 < Length) {
> -    List[i] = i;
> -  }
> -
> -#pragma clang loop unroll(enable)
> -#pragma unroll(8)
> -  while (i - 4 < Length) {
> -    List[i] = i;
> -  }
> -
> -#pragma unroll
> -#pragma clang loop unroll_count(4)
> -  while (i - 5 < Length) {
> -    List[i] = i;
> -  }
> -
>  /* expected-error {{expected ')'}} */ #pragma unroll(4
> -/* expected-error {{missing argument to '#pragma unroll'; expected a positive integer value}} */ #pragma unroll()
> +/* expected-error {{missing argument to '#pragma unroll'}} */ #pragma unroll()
>  /* expected-warning {{extra tokens at end of '#pragma unroll'}} */ #pragma unroll 1 2
>    while (i-6 < Length) {
>      List[i] = i;
> @@ -67,6 +49,18 @@
>      List[i] = i;
>    }
>
> +/* 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 '#pragma unroll' and '#pragma unroll(4)'}} */ #pragma unroll(4)
> +#pragma unroll
> +  while (i-11 < Length) {
> +    List[i] = i;
> +  }
> +
>  /* expected-error {{duplicate directives '#pragma unroll' and '#pragma unroll'}} */ #pragma unroll
>  #pragma unroll
>    while (i-14 < Length) {
> @@ -73,8 +67,8 @@
>      List[i] = i;
>    }
>
> -/* expected-error {{duplicate directives 'unroll(enable)' and '#pragma unroll'}} */ #pragma unroll
> -#pragma clang loop unroll(enable)
> +/* expected-error {{duplicate directives 'unroll(full)' and '#pragma unroll'}} */ #pragma unroll
> +#pragma clang loop unroll(full)
>    while (i-15 < Length) {
>      List[i] = i;
>    }
>

~Aaron



More information about the cfe-commits mailing list