[PATCH] Add loop unroll pragma support

[Aaron Ballman]

> Index: lib/Parse/ParsePragma.cpp
> ===================================================================
> --- lib/Parse/ParsePragma.cpp
> +++ lib/Parse/ParsePragma.cpp
> @@ -1641,8 +1641,10 @@
>  ///  loop-hint:
>  ///    'vectorize' '(' loop-hint-keyword ')'
>  ///    'interleave' '(' loop-hint-keyword ')'
> +///    'unroll' '(' loop-hint-keyword ')'
>  ///    'vectorize_width' '(' loop-hint-value ')'
>  ///    'interleave_count' '(' loop-hint-value ')'
> +///    'unroll_count' '(' loop-hint-value ')'
>  ///
>  ///  loop-hint-keyword:
>  ///    'enable'
> @@ -1661,6 +1663,13 @@
>  /// 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(enable) 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) {
> @@ -1680,8 +1689,9 @@
>      IdentifierInfo *OptionInfo = Tok.getIdentifierInfo();
>
>      if (!OptionInfo->isStr("vectorize") && !OptionInfo->isStr("interleave") &&
> -        !OptionInfo->isStr("vectorize_width") &&
> -        !OptionInfo->isStr("interleave_count")) {
> +        !OptionInfo->isStr("unroll") && !OptionInfo->isStr("vectorize_width") &&
> +        !OptionInfo->isStr("interleave_count") &&
> +        !OptionInfo->isStr("unroll_count")) {

At this point, I think a StringSwitch would make more sense.

> Index: lib/Sema/SemaStmtAttr.cpp
> ===================================================================
> --- lib/Sema/SemaStmtAttr.cpp
> +++ lib/Sema/SemaStmtAttr.cpp
> @@ -67,10 +67,13 @@
>            .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);
>
>    int ValueInt;
> -  if (Option == LoopHintAttr::Vectorize || Option == LoopHintAttr::Interleave) {
> +  if (Option == LoopHintAttr::Vectorize || Option == LoopHintAttr::Interleave ||
> +      Option == LoopHintAttr::Unroll) {
>      if (!ValueInfo) {
>        S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_keyword)
>            << /*MissingKeyword=*/true << "";
> @@ -87,7 +90,8 @@
>        return nullptr;
>      }
>    } else if (Option == LoopHintAttr::VectorizeWidth ||
> -             Option == LoopHintAttr::InterleaveCount) {
> +             Option == LoopHintAttr::InterleaveCount ||
> +             Option == LoopHintAttr::UnrollCount) {
>      // FIXME: We should support template parameters for the loop hint value.
>      // See bug report #19610.
>      llvm::APSInt ValueAPS;
> @@ -111,10 +115,26 @@
>
>  static void
>  CheckForIncompatibleAttributes(Sema &S, SmallVectorImpl<const Attr *> &Attrs) {
> -  int PrevOptionValue[4] = {-1, -1, -1, -1};
> -  int OptionId[4] = {LoopHintAttr::Vectorize, LoopHintAttr::VectorizeWidth,
> -                     LoopHintAttr::Interleave, LoopHintAttr::InterleaveCount};
> +  // 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 arrays
> +  // accumulate the hints encountered while iterating through the
> +  // attributes to check for compatibility
> +  //
> +  // Accumulated state of enable|disable hints for each hint category.
> +  bool EnabledIsSet[3] = {false, false, false};
> +  int EnabledValue[3];
> +  // Accumulated state of numeric hints for each hint category.
> +  bool NumericValueIsSet[3] = {false, false, false};
> +  int NumericValue[3];
>
> +  int EnableOptionId[3] = {LoopHintAttr::Vectorize, LoopHintAttr::Interleave,
> +                           LoopHintAttr::Unroll};
> +  int NumericOptionId[3] = {LoopHintAttr::VectorizeWidth,
> +                            LoopHintAttr::InterleaveCount,
> +                            LoopHintAttr::UnrollCount};

I agree with Reid's comments about perhaps making this into a struct
instead of parallel arrays. Also, the array size isn't needed.

Aside from these nits, LGTM, thanks!

~Aaron

http://reviews.llvm.org/D4089