[PATCH] Support constant expressions, including non-type template parameters, in pragma loop hints

[Aaron Ballman]

On Wed, Jul 30, 2014 at 8:49 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
> Hi Aaron,
>
> Thanks for the suggestions again. Here is the updated patch.
>
> #include "clang/AST/Decl.h"
> +#include "clang/AST/Expr.h"
>
>
> This include should not be required here.
>
>
> This is required to call printPretty on the value expression in
> LoopHintAttr.
>
>
> Ugh. That's unfortunate. Perhaps ClangAttrEmitter needs to understand
> that some AdditionalMembers are meant for the class declaration
> (Attrs.inc), and others are meant for the class definition
> (AttrImpl.inc)…
>
>
> Looks like that might be tricky. I think the easiest way to do that would be
> to modify tablegen to use an AdditionalMembersImpl field that would contain
> the implementation of methods defined in the AdditionalMembers field.
>
> I can try to do that before I leave, but with only 2 more days it is
> unlikely that I will be able to finish it.
>
>
> I was thinking of parsing nonsense (like a fuzzer):
>
> #pragma clang loop vectorize_width(1+(^*/2 * ()
> #pragma clang loop vectorize_width(1+(-0[0]))))))
>
> Just to ensure that we always recover gracefully from crazy input.
>
>
> I added the suggested tests. No problems.

> Index: include/clang/AST/Attr.h
> ===================================================================
> --- include/clang/AST/Attr.h (revision 214369)
> +++ include/clang/AST/Attr.h (working copy)
> @@ -16,6 +16,7 @@
>
>  #include "clang/AST/AttrIterator.h"
>  #include "clang/AST/Decl.h"
> +#include "clang/AST/Expr.h"
>  #include "clang/AST/Type.h"
>  #include "clang/Basic/AttrKinds.h"
>  #include "clang/Basic/LLVM.h"
> Index: include/clang/Basic/Attr.td
> ===================================================================
> --- include/clang/Basic/Attr.td (revision 214369)
> +++ include/clang/Basic/Attr.td (working copy)
> @@ -1809,7 +1809,7 @@
>                EnumArgument<"State", "LoopHintState",
>                             ["default", "enable", "disable"],
>                             ["Default", "Enable", "Disable"]>,
> -              DefaultIntArgument<"Value", 1>];
> +              ExprArgument<"Value">];
>
>    let AdditionalMembers = [{
>    static const char *getOptionName(int Option) {
> @@ -1849,7 +1849,7 @@
>      OS << "(";
>      if (option == VectorizeWidth || option == InterleaveCount ||
>          option == UnrollCount)
> -      OS << value;
> +      value->printPretty(OS, nullptr, Policy);
>      else if (state == Default)
>        return "";
>      else if (state == Enable)
> Index: include/clang/Basic/DiagnosticParseKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticParseKinds.td (revision 214369)
> +++ include/clang/Basic/DiagnosticParseKinds.td (working copy)
> @@ -925,8 +925,8 @@
>  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_loop_numeric_value : Error<
> -  "invalid argument; expected a positive integer value">;
> +def err_pragma_loop_invalid_value : Error<
> +  "%select{invalid|missing}0 argument; expected a positive 32-bit integer value">;
>
>  // Pragma unroll support.
>  def warn_pragma_unroll_cuda_value_in_parens : Warning<
> Index: include/clang/Basic/DiagnosticSemaKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSemaKinds.td (revision 214369)
> +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
> @@ -539,8 +539,10 @@
>    "#pragma visibility pop with no matching #pragma visibility push">;
>  def note_surrounding_namespace_starts_here : Note<
>    "surrounding namespace with visibility attribute starts here">;
> -def err_pragma_loop_invalid_value : Error<
> -  "invalid argument; expected a positive integer value">;
> +def err_pragma_loop_invalid_argument : Error<
> +  "invalid argument %0; expected a positive 32-bit integer value">;

Why is this diagnostic now in Sema and Parser (in the patch attached
to your latest email)?

> +def err_pragma_loop_invalid_expression : Error<
> +  "invalid expression; expected an integer constant expression">;
>  def err_pragma_loop_compatibility : Error<
>    "%select{incompatible|duplicate}0 directives '%1' and '%2'">;
>  def err_pragma_loop_precedes_nonloop : Error<
> Index: include/clang/Sema/Sema.h
> ===================================================================
> --- include/clang/Sema/Sema.h (revision 214369)
> +++ include/clang/Sema/Sema.h (working copy)
> @@ -3446,6 +3446,9 @@
>                                   PredefinedExpr::IdentType IT);
>    ExprResult ActOnPredefinedExpr(SourceLocation Loc, tok::TokenKind Kind);
>    ExprResult ActOnIntegerConstant(SourceLocation Loc, uint64_t Val);
> +
> +  bool CheckLoopHintExpr(Expr *E, SourceLocation Loc, bool AllowValueDependent);
> +
>    ExprResult ActOnNumericConstant(const Token &Tok, Scope *UDLScope = nullptr);
>    ExprResult ActOnCharacterConstant(const Token &Tok,
>                                      Scope *UDLScope = nullptr);
>

~Aaron