Addressed all of your comments. The formatting issues were due to using clang-format.py without the "-style LLVM" option.<br><br>Committed as r213771 and r213772.<br><div class="gmail_quote">On Tue Jul 22 2014 at 12:06:58 PM, Aaron Ballman <<a href="mailto:aaron.ballman@gmail.com">aaron.ballman@gmail.com</a>> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Tue, Jul 22, 2014 at 2:33 PM, Mark Heffernan <<a href="mailto:meheff@google.com" target="_blank">meheff@google.com</a>> wrote:<br>
> This patch does some renaming of the metadata and unrolling pragma terms to<br>
> hopefully remove a source of confusion. It was motivated the comments in<br>
> review: <a href="http://reviews.llvm.org/D4571" target="_blank">http://reviews.llvm.org/D4571</a>. Prior to this patch we have the<br>
> following pragma syntax and the corresponding metadata it generates:<br>
><br>
> #pragma clang loop unroll(enable) => !{metadata !"llvm.loop.unroll.enable",<br>
> i1 true}<br>
> #pragma clang loop unroll(disable) => !{metadata<br>
> !"llvm.loop.unroll.enable", i1 false}<br>
> #pragma clang loop unroll_count(N) => !{metadata !"llvm.loop.unroll.count",<br>
> i32 N}<br>
><br>
> #pragma unroll => !{metadata !"llvm.loop.unroll.enable", i1 true}<br>
> #pragma unroll N => !{metadata !"llvm.loop.unroll.count", i32 N}<br>
><br>
> Unroll disable and unroll count are pretty self explanatory. The issue is<br>
> with "unroll(enable)". "unroll(enable)" indicates to unroll the loop fully<br>
> which is a bit surprising. It also is not clear whether you need<br>
> "unroll(enable)" when you specify "unroll_count(N)". Both of these<br>
> potential sources of confusion are resolved with this patch which changes<br>
> the pragma and metadata to:<br>
><br>
> #pragma clang loop unroll(full) => !{metadata !"llvm.loop.unroll.full"}<br>
> #pragma clang loop unroll(disable) => !{metadata<br>
> !"llvm.loop.unroll.disable"}<br>
> #pragma clang loop unroll_count(N) => !{metadata !"llvm.loop.unroll.count",<br>
> i32 N}<br>
><br>
> #pragma unroll => !{metadata !"llvm.loop.unroll.full"}<br>
> #pragma unroll N => !{metadata !"llvm.loop.unroll.count", i32 N}<br>
><br>
> Now it should (hopefully) be immediately clear what each of these do. The<br>
> loop unrolling transformation itself is not affected. Other than renaming,<br>
> the only functional difference is that a loop can now only have a single<br>
> loop unroll directive. Previously (and confusingly) a loop could have both<br>
> unroll(enable) and unroll_count(N).<br>
<br>
Some very minor comments below on the clang side of the patch. Aside<br>
from these nits, LGTM.<br>
<br>
> Index: docs/LanguageExtensions.rst<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- docs/LanguageExtensions.rst (revision 213580)<br>
> +++ docs/LanguageExtensions.rst (working copy)<br>
> @@ -1830,7 +1830,7 @@<br>
> compile time. Partial unrolling replicates the loop body within the loop and<br>
> reduces the trip count.<br>
><br>
> -If ``unroll(enable)`` is specified the unroller will attempt to fully unroll the<br>
> +If ``unroll(full)`` is specified the unroller will attempt to fully unroll the<br>
> loop if the trip count is known at compile time. If the loop count is not known<br>
> or the fully unrolled code size is greater than the limit specified by the<br>
> `-pragma-unroll-threshold` command line option the loop will be partially<br>
> @@ -1838,7 +1838,7 @@<br>
><br>
> .. code-block:: c++<br>
><br>
> - #pragma clang loop unroll(enable)<br>
> + #pragma clang loop unroll(full)<br>
> for(...) {<br>
> ...<br>
> }<br>
> Index: include/clang/Basic/Attr.td<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- include/clang/Basic/Attr.td (revision 213580)<br>
> +++ include/clang/Basic/Attr.td (working copy)<br>
> @@ -1776,7 +1776,7 @@<br>
> /// vectorize_width: vectorize loop operations with width 'value'.<br>
> /// interleave: interleave multiple loop iterations if 'value != 0'.<br>
> /// interleave_count: interleaves 'value' loop interations.<br>
> - /// unroll: unroll loop if 'value != 0'.<br>
> + /// unroll: fully unroll loop if 'value != 0'.<br>
> /// unroll_count: unrolls loop 'value' times.<br>
><br>
> let Spellings = [Pragma<"clang", "loop">, Pragma<"", "unroll">];<br>
> @@ -1830,6 +1830,10 @@<br>
> if (option == VectorizeWidth || option == InterleaveCount ||<br>
> option == UnrollCount)<br>
> OS << value;<br>
> + else if (option == Unroll && value)<br>
> + // Unroll loop hint does not use the keyword "enable". Instead, a true value<br>
> + // indicate full unrolling which uses the keyword "full".<br>
<br>
Typo: indicates? Also, instead of "true value", perhaps a "nonzero<br>
value" would read a bit easier, since these are integer arguments.<br>
<br>
> + OS << "full";<br>
> else if (value)<br>
> OS << "enable";<br>
> else<br>
> Index: include/clang/Basic/AttrDocs.<u></u>td<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- include/clang/Basic/AttrDocs.<u></u>td (revision 213580)<br>
> +++ include/clang/Basic/AttrDocs.<u></u>td (working copy)<br>
> @@ -1080,7 +1080,7 @@<br>
> }<br>
><br>
> ``#pragma unroll`` and ``#pragma unroll _value_`` have identical semantics to<br>
> -``#pragma clang loop unroll(enable)`` and ``#pragma clang loop<br>
> +``#pragma clang loop unroll(full)`` and ``#pragma clang loop<br>
> unroll_count(_value_)`` respectively. See `language extensions<br>
> <<a href="http://clang.llvm.org/docs/LanguageExtensions.html#extensions-for-loop-hint-optimizations" target="_blank">http://clang.llvm.org/docs/<u></u>LanguageExtensions.html#<u></u>extensions-for-loop-hint-<u></u>optimizations</a>>`_<br>
> for further details including limitations of the unroll hints.<br>
> Index: include/clang/Basic/<u></u>DiagnosticParseKinds.td<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- include/clang/Basic/<u></u>DiagnosticParseKinds.td (revision 213580)<br>
> +++ include/clang/Basic/<u></u>DiagnosticParseKinds.td (working copy)<br>
> @@ -815,7 +815,7 @@<br>
> "expected non-wide string literal in '#pragma %0'">, InGroup<IgnoredPragmas>;<br>
> // - Generic errors<br>
> def err_pragma_missing_argument : Error<<br>
> - "missing argument to '#pragma %0'; expected %1">;<br>
> + "missing argument to '#pragma %0'%select{|; expected %2}1">;<br>
> // - #pragma options<br>
> def warn_pragma_options_expected_<u></u>align : Warning<<br>
> "expected 'align' following '#pragma options' - ignored">,<br>
> Index: include/clang/Basic/<u></u>DiagnosticSemaKinds.td<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- include/clang/Basic/<u></u>DiagnosticSemaKinds.td (revision 213580)<br>
> +++ include/clang/Basic/<u></u>DiagnosticSemaKinds.td (working copy)<br>
> @@ -539,7 +539,7 @@<br>
> def err_pragma_loop_invalid_value : Error<<br>
> "invalid argument; expected a positive integer value">;<br>
> def err_pragma_loop_invalid_<u></u>keyword : Error<<br>
> - "invalid argument; expected 'enable' or 'disable'">;<br>
> + "invalid argument; expected '%0' or 'disable'">;<br>
> def err_pragma_loop_compatibility : Error<<br>
> "%select{incompatible|<u></u>duplicate}0 directives '%1' and '%2'">;<br>
> def err_pragma_loop_precedes_<u></u>nonloop : Error<<br>
> Index: lib/CodeGen/CGStmt.cpp<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- lib/CodeGen/CGStmt.cpp (revision 213580)<br>
> +++ lib/CodeGen/CGStmt.cpp (working copy)<br>
> @@ -586,55 +586,53 @@<br>
><br>
> const char *MetadataName;<br>
> switch (Option) {<br>
> - case LoopHintAttr::Vectorize:<br>
> - case LoopHintAttr::VectorizeWidth:<br>
> - MetadataName = "llvm.loop.vectorize.width";<br>
> - break;<br>
> - case LoopHintAttr::Interleave:<br>
> - case LoopHintAttr::InterleaveCount:<br>
> - MetadataName = "llvm.loop.vectorize.unroll";<br>
> - break;<br>
> - case LoopHintAttr::Unroll:<br>
> - MetadataName = "llvm.loop.unroll.enable";<br>
> - break;<br>
> - case LoopHintAttr::UnrollCount:<br>
> - MetadataName = "llvm.loop.unroll.count";<br>
> - break;<br>
> + case LoopHintAttr::Vectorize:<br>
> + case LoopHintAttr::VectorizeWidth:<br>
> + MetadataName = "llvm.loop.vectorize.width";<br>
> + break;<br>
> + case LoopHintAttr::Interleave:<br>
> + case LoopHintAttr::InterleaveCount:<br>
> + MetadataName = "llvm.loop.vectorize.unroll";<br>
> + break;<br>
> + case LoopHintAttr::Unroll:<br>
> + // With the unroll loop hint, a non-zero value indicates full unrolling.<br>
> + MetadataName = ValueInt == 0 ? "llvm.loop.unroll.disable"<br>
> + : "llvm.loop.unroll.full";<br>
> + break;<br>
> + case LoopHintAttr::UnrollCount:<br>
> + MetadataName = "llvm.loop.unroll.count";<br>
> + break;<br>
<br>
The formatting here does not match our style guides.<br>
<br>
> }<br>
> -<br>
> llvm::Value *Value;<br>
> llvm::MDString *Name;<br>
> switch (Option) {<br>
> - case LoopHintAttr::Vectorize:<br>
> - case LoopHintAttr::Interleave:<br>
> - if (ValueInt == 1) {<br>
> - // FIXME: In the future I will modifiy the behavior of the metadata<br>
> - // so we can enable/disable vectorization and interleaving separately.<br>
> - Name = llvm::MDString::get(Context, "llvm.loop.vectorize.enable");<br>
> - Value = Builder.getTrue();<br>
> + case LoopHintAttr::Vectorize:<br>
> + case LoopHintAttr::Interleave:<br>
> + if (ValueInt == 1) {<br>
> + // FIXME: In the future I will modifiy the behavior of the metadata<br>
> + // so we can enable/disable vectorization and interleaving separately.<br>
> + Name = llvm::MDString::get(Context, "llvm.loop.vectorize.enable");<br>
> + Value = Builder.getTrue();<br>
> + break;<br>
> + }<br>
> + // Vectorization/interleaving is disabled, set width/count to 1.<br>
> + ValueInt = 1;<br>
> + // Fallthrough.<br>
> + case LoopHintAttr::VectorizeWidth:<br>
> + case LoopHintAttr::InterleaveCount:<br>
> + case LoopHintAttr::UnrollCount:<br>
> + Name = llvm::MDString::get(Context, MetadataName);<br>
> + Value = llvm::ConstantInt::get(<u></u>Int32Ty, ValueInt);<br>
> break;<br>
> - }<br>
> - // Vectorization/interleaving is disabled, set width/count to 1.<br>
> - ValueInt = 1;<br>
> - // Fallthrough.<br>
> - case LoopHintAttr::VectorizeWidth:<br>
> - case LoopHintAttr::InterleaveCount:<br>
> - Name = llvm::MDString::get(Context, MetadataName);<br>
> - Value = llvm::ConstantInt::get(<u></u>Int32Ty, ValueInt);<br>
> - break;<br>
> - case LoopHintAttr::Unroll:<br>
> - Name = llvm::MDString::get(Context, MetadataName);<br>
> - Value = (ValueInt == 0) ? Builder.getFalse() : Builder.getTrue();<br>
> - break;<br>
> - case LoopHintAttr::UnrollCount:<br>
> - Name = llvm::MDString::get(Context, MetadataName);<br>
> - Value = llvm::ConstantInt::get(<u></u>Int32Ty, ValueInt);<br>
> - break;<br>
> + case LoopHintAttr::Unroll:<br>
> + Name = llvm::MDString::get(Context, MetadataName);<br>
> + Value = nullptr;<br>
> + break;<br>
<br>
Same issue with formatting here as above.<br>
<br>
> }<br>
><br>
> SmallVector<llvm::Value *, 2> OpValues;<br>
> OpValues.push_back(Name);<br>
> - OpValues.push_back(Value);<br>
> + if (Value) OpValues.push_back(Value);<br>
<br>
The controlled statement should be on its own line.<br>
<br>
><br>
> // Set or overwrite metadata indicated by Name.<br>
> Metadata.push_back(llvm::<u></u>MDNode::get(Context, OpValues));<br>
> Index: lib/Parse/ParsePragma.cpp<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- lib/Parse/ParsePragma.cpp (revision 213580)<br>
> +++ lib/Parse/ParsePragma.cpp (working copy)<br>
> @@ -1670,8 +1670,7 @@<br>
> PP.Lex(Tok);<br>
> if (Tok.is(tok::eod)) {<br>
> PP.Diag(Tok.getLocation(), diag::err_pragma_missing_<u></u>argument)<br>
> - << "clang optimize"<br>
> - << "'on' or 'off'";<br>
> + << "clang optimize" << /*Expected=*/true << "'on' or 'off'";<br>
> return;<br>
> }<br>
> if (Tok.isNot(tok::identifier)) {<br>
> @@ -1718,8 +1717,12 @@<br>
> "Unexpected pragma name");<br>
> PragmaString = "unroll";<br>
> }<br>
> + // Don't try to emit what the pragma is expecting with the diagnostic<br>
> + // because the logic is non-trivial and we give expected values in sema<br>
> + // diagnostics if an invalid argument is given. Here, just note that the<br>
> + // pragma is missing an argument.<br>
> PP.Diag(Tok.getLocation(), diag::err_pragma_missing_<u></u>argument)<br>
> - << PragmaString << "a positive integer value";<br>
> + << PragmaString << /*Expected=*/false;<br>
> return true;<br>
> }<br>
> }<br>
> @@ -1751,7 +1754,7 @@<br>
> /// loop-hint:<br>
> /// 'vectorize' '(' loop-hint-keyword ')'<br>
> /// 'interleave' '(' loop-hint-keyword ')'<br>
> -/// 'unroll' '(' loop-hint-keyword ')'<br>
> +/// 'unroll' '(' unroll-hint-keyword ')'<br>
> /// 'vectorize_width' '(' loop-hint-value ')'<br>
> /// 'interleave_count' '(' loop-hint-value ')'<br>
> /// 'unroll_count' '(' loop-hint-value ')'<br>
> @@ -1760,6 +1763,10 @@<br>
> /// 'enable'<br>
> /// 'disable'<br>
> ///<br>
> +/// unroll-hint-keyword:<br>
> +/// 'full'<br>
> +/// 'disable'<br>
> +///<br>
> /// loop-hint-value:<br>
> /// constant-expression<br>
> ///<br>
> @@ -1774,12 +1781,10 @@<br>
> /// only works on inner loops.<br>
> ///<br>
> /// The unroll and unroll_count directives control the concatenation<br>
> -/// unroller. Specifying unroll(enable) instructs llvm to try to<br>
> +/// unroller. Specifying unroll(full) instructs llvm to try to<br>
> /// unroll the loop completely, and unroll(disable) disables unrolling<br>
> /// for the loop. Specifying unroll_count(_value_) instructs llvm to<br>
> /// try to unroll the loop the number of times indicated by the value.<br>
> -/// If unroll(enable) and unroll_count are both specified only<br>
> -/// unroll_count takes effect.<br>
> void PragmaLoopHintHandler::<u></u>HandlePragma(Preprocessor &PP,<br>
> PragmaIntroducerKind Introducer,<br>
> Token &Tok) {<br>
> Index: lib/Sema/SemaStmtAttr.cpp<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- lib/Sema/SemaStmtAttr.cpp (revision 213580)<br>
> +++ lib/Sema/SemaStmtAttr.cpp (working copy)<br>
> @@ -89,16 +89,22 @@<br>
> } else if (Option == LoopHintAttr::Vectorize ||<br>
> Option == LoopHintAttr::Interleave ||<br>
> Option == LoopHintAttr::Unroll) {<br>
> + // Unrolling uses the keyword "full" rather than "enable" to indicate full<br>
> + // unrolling.<br>
> + const char *TrueKeyword =<br>
> + Option == LoopHintAttr::Unroll ? "full" : "enable";<br>
> if (!ValueInfo) {<br>
> - S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_<u></u>keyword);<br>
> + S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_<u></u>keyword)<br>
> + << TrueKeyword;<br>
> return nullptr;<br>
> }<br>
> if (ValueInfo->isStr("disable"))<br>
> ValueInt = 0;<br>
> - else if (ValueInfo->isStr("enable"))<br>
> + else if (ValueInfo->getName() == TrueKeyword)<br>
> ValueInt = 1;<br>
> else {<br>
> - S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_<u></u>keyword);<br>
> + S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_<u></u>keyword)<br>
> + << TrueKeyword;<br>
> return nullptr;<br>
> }<br>
> } else if (Option == LoopHintAttr::VectorizeWidth ||<br>
> @@ -121,12 +127,14 @@<br>
><br>
> static void CheckForIncompatibleAttributes<u></u>(<br>
> Sema &S, const SmallVectorImpl<const Attr *> &Attrs) {<br>
> - // There are 3 categories of loop hints: vectorize, interleave, and<br>
> - // unroll. Each comes in two variants: an enable/disable form and a<br>
> - // form which takes a numeric argument. For example:<br>
> - // unroll(enable|disable) and unroll_count(N). The following array<br>
> - // accumulate the hints encountered while iterating through the<br>
> - // attributes to check for compatibility.<br>
> + // There are 3 categories of loop hints attributes: vectorize, interleave, and<br>
> + // unroll. Each comes in two variants: a boolean form and a numeric form. The<br>
> + // boolean hints selectively enables/disables the transformation for the loop<br>
> + // (for unroll a true value indicates full unrolling rather than enabling the<br>
<br>
Instead of true value, perhaps "nonzero value"? Also, missing a comma<br>
after "unroll."<br>
<br>
> + // transformation). The numeric hint provides an integer hint (for example,<br>
> + // unroll count) to the transformer. The following array accumulate the hints<br>
<br>
Pluralization mismatch (array accumulate).<br>
<br>
> + // encountered while iterating through the attributes to check for<br>
> + // compatibility.<br>
> struct {<br>
> const LoopHintAttr *EnableAttr;<br>
> const LoopHintAttr *NumericAttr;<br>
> @@ -141,19 +149,20 @@<br>
><br>
> int Option = LH->getOption();<br>
> int Category;<br>
> + enum { Vectorize, Interleave, Unroll };<br>
> switch (Option) {<br>
> - case LoopHintAttr::Vectorize:<br>
> - case LoopHintAttr::VectorizeWidth:<br>
> - Category = 0;<br>
> - break;<br>
> - case LoopHintAttr::Interleave:<br>
> - case LoopHintAttr::InterleaveCount:<br>
> - Category = 1;<br>
> - break;<br>
> - case LoopHintAttr::Unroll:<br>
> - case LoopHintAttr::UnrollCount:<br>
> - Category = 2;<br>
> - break;<br>
> + case LoopHintAttr::Vectorize:<br>
> + case LoopHintAttr::VectorizeWidth:<br>
> + Category = Vectorize;<br>
> + break;<br>
> + case LoopHintAttr::Interleave:<br>
> + case LoopHintAttr::InterleaveCount:<br>
> + Category = Interleave;<br>
> + break;<br>
> + case LoopHintAttr::Unroll:<br>
> + case LoopHintAttr::UnrollCount:<br>
> + Category = Unroll;<br>
> + break;<br>
<br>
Formatting.<br>
<br>
> };<br>
><br>
> auto &CategoryState = HintAttrs[Category];<br>
> @@ -176,10 +185,11 @@<br>
> << /*Duplicate=*/true << PrevAttr->getDiagnosticName()<br>
> << LH->getDiagnosticName();<br>
><br>
> - if (CategoryState.EnableAttr && !CategoryState.EnableAttr-><u></u>getValue() &&<br>
> - CategoryState.NumericAttr) {<br>
> - // Disable hints are not compatible with numeric hints of the<br>
> - // same category.<br>
> + if (CategoryState.EnableAttr && CategoryState.NumericAttr &&<br>
> + (Category == Unroll || !CategoryState.EnableAttr-><u></u>getValue())) {<br>
> + // Disable hints are not compatible with numeric hints of the same<br>
> + // category. As a special case, numeric unroll hints are also not<br>
> + // compatible with "enable" form of the unroll pragma, unroll(full).<br>
> S.Diag(OptionLoc, diag::err_pragma_loop_<u></u>compatibility)<br>
> << /*Duplicate=*/false<br>
> << CategoryState.EnableAttr-><u></u>getDiagnosticName()<br>
> Index: test/CodeGen/pragma-loop.cpp<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- test/CodeGen/pragma-loop.cpp (revision 213580)<br>
> +++ test/CodeGen/pragma-loop.cpp (working copy)<br>
> @@ -8,7 +8,7 @@<br>
> #pragma clang loop vectorize(enable)<br>
> #pragma clang loop interleave_count(4)<br>
> #pragma clang loop vectorize_width(4)<br>
> -#pragma clang loop unroll(enable)<br>
> +#pragma clang loop unroll(full)<br>
> while (i < Length) {<br>
> // CHECK: br i1 {{.*}}, label {{.*}}, label {{.*}}, !llvm.loop ![[LOOP_1:.*]]<br>
> List[i] = i * 2;<br>
> @@ -108,13 +108,13 @@<br>
> for_template_define_test<<u></u>double>(List, Length, Value);<br>
> }<br>
><br>
> -// CHECK: ![[LOOP_1]] = metadata !{metadata ![[LOOP_1]], metadata ![[UNROLLENABLE_1:.*]], metadata ![[WIDTH_4:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[INTENABLE_1:.*]]}<br>
> -// CHECK: ![[UNROLLENABLE_1]] = metadata !{metadata !"llvm.loop.unroll.enable", i1 true}<br>
> +// CHECK: ![[LOOP_1]] = metadata !{metadata ![[LOOP_1]], metadata ![[UNROLL_FULL:.*]], metadata ![[WIDTH_4:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[INTENABLE_1:.*]]}<br>
> +// CHECK: ![[UNROLL_FULL]] = metadata !{metadata !"llvm.loop.unroll.full"}<br>
> // CHECK: ![[WIDTH_4]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 4}<br>
> // CHECK: ![[INTERLEAVE_4]] = metadata !{metadata !"llvm.loop.vectorize.unroll", i32 4}<br>
> // CHECK: ![[INTENABLE_1]] = metadata !{metadata !"llvm.loop.vectorize.enable", i1 true}<br>
> -// CHECK: ![[LOOP_2]] = metadata !{metadata ![[LOOP_2:.*]], metadata ![[UNROLLENABLE_0:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[WIDTH_8:.*]]}<br>
> -// CHECK: ![[UNROLLENABLE_0]] = metadata !{metadata !"llvm.loop.unroll.enable", i1 false}<br>
> +// CHECK: ![[LOOP_2]] = metadata !{metadata ![[LOOP_2:.*]], metadata ![[UNROLL_DISABLE:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[WIDTH_8:.*]]}<br>
> +// CHECK: ![[UNROLL_DISABLE]] = metadata !{metadata !"llvm.loop.unroll.disable"}<br>
> // CHECK: ![[WIDTH_8]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 8}<br>
> // CHECK: ![[LOOP_3]] = metadata !{metadata ![[LOOP_3]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_4:.*]], metadata ![[ENABLE_1:.*]]}<br>
> // CHECK: ![[UNROLL_8]] = metadata !{metadata !"llvm.loop.unroll.count", i32 8}<br>
> @@ -121,7 +121,7 @@<br>
> // CHECK: ![[LOOP_4]] = metadata !{metadata ![[LOOP_4]], metadata ![[INTERLEAVE_2:.*]], metadata ![[WIDTH_2:.*]]}<br>
> // CHECK: ![[INTERLEAVE_2]] = metadata !{metadata !"llvm.loop.vectorize.unroll", i32 2}<br>
> // CHECK: ![[WIDTH_2]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 2}<br>
> -// CHECK: ![[LOOP_5]] = metadata !{metadata ![[LOOP_5]], metadata ![[UNROLLENABLE_0:.*]], metadata ![[WIDTH_1:.*]]}<br>
> +// CHECK: ![[LOOP_5]] = metadata !{metadata ![[LOOP_5]], metadata ![[UNROLL_DISABLE:.*]], metadata ![[WIDTH_1:.*]]}<br>
> // CHECK: ![[WIDTH_1]] = metadata !{metadata !"llvm.loop.vectorize.width", i32 1}<br>
> // CHECK: ![[LOOP_6]] = metadata !{metadata ![[LOOP_6]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_2:.*]], metadata ![[WIDTH_2:.*]]}<br>
> // CHECK: ![[LOOP_7]] = metadata !{metadata ![[LOOP_7]], metadata ![[UNROLL_8:.*]], metadata ![[INTERLEAVE_8:.*]], metadata ![[WIDTH_8:.*]]}<br>
> Index: test/CodeGen/pragma-unroll.cpp<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- test/CodeGen/pragma-unroll.cpp (revision 213580)<br>
> +++ test/CodeGen/pragma-unroll.cpp (working copy)<br>
> @@ -86,8 +86,8 @@<br>
> for_template_define_test<<u></u>double>(List, Length, Value);<br>
> }<br>
><br>
> -// CHECK: ![[LOOP_1]] = metadata !{metadata ![[LOOP_1]], metadata ![[UNROLLENABLE_1:.*]]}<br>
> -// CHECK: ![[UNROLLENABLE_1]] = metadata !{metadata !"llvm.loop.unroll.enable", i1 true}<br>
> +// CHECK: ![[LOOP_1]] = metadata !{metadata ![[LOOP_1]], metadata ![[UNROLL_FULL:.*]]}<br>
> +// CHECK: ![[UNROLL_FULL]] = metadata !{metadata !"llvm.loop.unroll.full"}<br>
> // CHECK: ![[LOOP_2]] = metadata !{metadata ![[LOOP_2:.*]], metadata ![[UNROLL_16:.*]]}<br>
> // CHECK: ![[UNROLL_16]] = metadata !{metadata !"llvm.loop.unroll.count", i32 16}<br>
> // CHECK: ![[LOOP_3]] = metadata !{metadata ![[LOOP_3]], metadata ![[UNROLL_8:.*]]}<br>
> Index: test/PCH/pragma-loop.cpp<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- test/PCH/pragma-loop.cpp (revision 213580)<br>
> +++ test/PCH/pragma-loop.cpp (working copy)<br>
> @@ -10,7 +10,7 @@<br>
> // CHECK: #pragma clang loop unroll(disable)<br>
> // CHECK: #pragma clang loop interleave(disable)<br>
> // CHECK: #pragma clang loop vectorize(enable)<br>
> -// CHECK: #pragma clang loop unroll(enable)<br>
> +// CHECK: #pragma clang loop unroll(full)<br>
> // CHECK: #pragma clang loop interleave(enable)<br>
> // CHECK: #pragma clang loop vectorize(disable)<br>
> // CHECK: #pragma unroll<br>
> @@ -47,7 +47,7 @@<br>
> int i = 0;<br>
> #pragma clang loop vectorize(disable)<br>
> #pragma clang loop interleave(enable)<br>
> -#pragma clang loop unroll(enable)<br>
> +#pragma clang loop unroll(full)<br>
> while (i - 3 < Length) {<br>
> List[i] = i;<br>
> i++;<br>
> Index: test/Parser/pragma-loop.cpp<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- test/Parser/pragma-loop.cpp (revision 213580)<br>
> +++ test/Parser/pragma-loop.cpp (working copy)<br>
> @@ -8,7 +8,7 @@<br>
><br>
> #pragma clang loop vectorize(enable)<br>
> #pragma clang loop interleave(enable)<br>
> -#pragma clang loop unroll(enable)<br>
> +#pragma clang loop unroll(full)<br>
> while (i + 1 < Length) {<br>
> List[i] = i;<br>
> }<br>
> @@ -49,15 +49,15 @@<br>
><br>
> /* expected-error {{expected ')'}} */ #pragma clang loop vectorize(enable<br>
> /* expected-error {{expected ')'}} */ #pragma clang loop interleave(enable<br>
> -/* expected-error {{expected ')'}} */ #pragma clang loop unroll(enable<br>
> +/* expected-error {{expected ')'}} */ #pragma clang loop unroll(full<br>
><br>
> /* expected-error {{expected ')'}} */ #pragma clang loop vectorize_width(4<br>
> /* expected-error {{expected ')'}} */ #pragma clang loop interleave_count(4<br>
> /* expected-error {{expected ')'}} */ #pragma clang loop unroll_count(4<br>
><br>
> -/* expected-error {{missing argument to '#pragma clang loop vectorize'; expected a positive integer value}} */ #pragma clang loop vectorize()<br>
> -/* expected-error {{missing argument to '#pragma clang loop interleave_count'; expected a positive integer value}} */ #pragma clang loop interleave_count()<br>
> -/* expected-error {{missing argument to '#pragma clang loop unroll'; expected a positive integer value}} */ #pragma clang loop unroll()<br>
> +/* expected-error {{missing argument to '#pragma clang loop vectorize'}} */ #pragma clang loop vectorize()<br>
> +/* expected-error {{missing argument to '#pragma clang loop interleave_count'}} */ #pragma clang loop interleave_count()<br>
> +/* expected-error {{missing argument to '#pragma clang loop unroll'}} */ #pragma clang loop unroll()<br>
><br>
> /* expected-error {{missing option}} */ #pragma clang loop<br>
> /* expected-error {{invalid option 'badkeyword'}} */ #pragma clang loop badkeyword<br>
> @@ -92,7 +92,7 @@<br>
><br>
> /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop vectorize(badidentifier)<br>
> /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop interleave(badidentifier)<br>
> -/* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop unroll(badidentifier)<br>
> +/* expected-error {{invalid argument; expected 'full' or 'disable'}} */ #pragma clang loop unroll(badidentifier)<br>
> while (i-7 < Length) {<br>
> List[i] = i;<br>
> }<br>
> @@ -101,7 +101,7 @@<br>
> // constants crash FE.<br>
> /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop vectorize(()<br>
> /* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop interleave(*)<br>
> -/* expected-error {{invalid argument; expected 'enable' or 'disable'}} */ #pragma clang loop unroll(=)<br>
> +/* expected-error {{invalid argument; expected 'full' or 'disable'}} */ #pragma clang loop unroll(=)<br>
> /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop vectorize_width(^)<br>
> /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop interleave_count(/)<br>
> /* expected-error {{invalid argument; expected a positive integer value}} */ #pragma clang loop unroll_count(==)<br>
> @@ -136,7 +136,7 @@<br>
> #pragma clang loop vectorize(disable)<br>
> /* expected-error {{duplicate directives 'interleave(disable)' and 'interleave(enable)'}} */ #pragma clang loop interleave(enable)<br>
> #pragma clang loop interleave(disable)<br>
> -/* expected-error {{duplicate directives 'unroll(disable)' and 'unroll(enable)'}} */ #pragma clang loop unroll(enable)<br>
> +/* expected-error {{duplicate directives 'unroll(disable)' and 'unroll(full)'}} */ #pragma clang loop unroll(full)<br>
> #pragma clang loop unroll(disable)<br>
> while (i-9 < Length) {<br>
> List[i] = i;<br>
> @@ -162,5 +162,12 @@<br>
> List[i] = i;<br>
> }<br>
><br>
> +<br>
> +/* expected-error {{incompatible directives 'unroll(full)' and 'unroll_count(4)'}} */ #pragma clang loop unroll(full)<br>
> +#pragma clang loop unroll_count(4)<br>
> + while (i-11 < Length) {<br>
> + List[i] = i;<br>
> + }<br>
> +<br>
> #pragma clang loop interleave(enable)<br>
> /* expected-error {{expected statement}} */ }<br>
> Index: test/Parser/pragma-unroll.cpp<br>
> ==============================<u></u>==============================<u></u>=======<br>
> --- test/Parser/pragma-unroll.cpp (revision 213580)<br>
> +++ test/Parser/pragma-unroll.cpp (working copy)<br>
> @@ -21,26 +21,8 @@<br>
> List[i] = i;<br>
> }<br>
><br>
> -#pragma unroll<br>
> -#pragma unroll(8)<br>
> - while (i - 3 < Length) {<br>
> - List[i] = i;<br>
> - }<br>
> -<br>
> -#pragma clang loop unroll(enable)<br>
> -#pragma unroll(8)<br>
> - while (i - 4 < Length) {<br>
> - List[i] = i;<br>
> - }<br>
> -<br>
> -#pragma unroll<br>
> -#pragma clang loop unroll_count(4)<br>
> - while (i - 5 < Length) {<br>
> - List[i] = i;<br>
> - }<br>
> -<br>
> /* expected-error {{expected ')'}} */ #pragma unroll(4<br>
> -/* expected-error {{missing argument to '#pragma unroll'; expected a positive integer value}} */ #pragma unroll()<br>
> +/* expected-error {{missing argument to '#pragma unroll'}} */ #pragma unroll()<br>
> /* expected-warning {{extra tokens at end of '#pragma unroll'}} */ #pragma unroll 1 2<br>
> while (i-6 < Length) {<br>
> List[i] = i;<br>
> @@ -67,6 +49,18 @@<br>
> List[i] = i;<br>
> }<br>
><br>
> +/* expected-error {{incompatible directives 'unroll(full)' and '#pragma unroll(4)'}} */ #pragma unroll(4)<br>
> +#pragma clang loop unroll(full)<br>
> + while (i-11 < Length) {<br>
> + List[i] = i;<br>
> + }<br>
> +<br>
> +/* expected-error {{incompatible directives '#pragma unroll' and '#pragma unroll(4)'}} */ #pragma unroll(4)<br>
> +#pragma unroll<br>
> + while (i-11 < Length) {<br>
> + List[i] = i;<br>
> + }<br>
> +<br>
> /* expected-error {{duplicate directives '#pragma unroll' and '#pragma unroll'}} */ #pragma unroll<br>
> #pragma unroll<br>
> while (i-14 < Length) {<br>
> @@ -73,8 +67,8 @@<br>
> List[i] = i;<br>
> }<br>
><br>
> -/* expected-error {{duplicate directives 'unroll(enable)' and '#pragma unroll'}} */ #pragma unroll<br>
> -#pragma clang loop unroll(enable)<br>
> +/* expected-error {{duplicate directives 'unroll(full)' and '#pragma unroll'}} */ #pragma unroll<br>
> +#pragma clang loop unroll(full)<br>
> while (i-15 < Length) {<br>
> List[i] = i;<br>
> }<br>
><br>
<br>
~Aaron<br>
</blockquote></div>