>>>
>>> ParsePragma.cpp:1619: // Read '('
>>> This looks like a good place to use BalancedDelimiterTracker for
parsing '(' and ')’.

>>I don’t think it is needed. It isn’t used by any other #pragma directives
and the syntax here is rather simple. What do you think would be the
benefit?

This would improve consistency with the other places in clang where '(' and
')' are parsed. Otherwise it seems to be equivalent to handling '(' and ')'
manually.


>>> It would be very useful for auto-tuning libraries certainly. I am not
>>> sure how to support it currently. I suggest we leave it for a future
>>> commit.

>>I suspect that Alexey and Alexander can provide advice here.

For openmp clauses, we have methods in TreeTransform for this (e.g.
TransformOMPSafelenClause).
You will probably need something similar for attributes.



2014-04-29 4:19 GMT+04:00 Hal Finkel <[email protected]>:

> ----- Original Message -----
> > From: "Tyler Nowicki" <[email protected]>
> > To: "Hal Finkel" <[email protected]>
> > Cc: "Alexey Bataev" <[email protected]>, "llvm cfe" <
> [email protected]>, "Nadav Rotem" <[email protected]>,
> > "Alexander Musman" <[email protected]>
> > Sent: Monday, April 28, 2014 7:09:13 PM
> > Subject: Re: [PATCH] #pragma vectorize
> >
> >
> > It would be very useful for auto-tuning libraries certainly. I am not
> > sure how to support it currently. I suggest we leave it for a future
> > commit.
>
> I suspect that Alexey and Alexander can provide advice here. I don't see
> how this is any different from the method used by the OpenMP pragrmas to
> support this... look at test/OpenMP/simd_ast_print.cpp and how the simdlen
> clause supports this.
>
> I think that part of the problem is that you're restricting the form of
> the value too early:
>
> The code:
>
> +    // Read the optimization value identifier.
>
> +    PP.Lex(Tok);
>
> +    if (Tok.isNot(tok::identifier) && Tok.isNot(tok::numeric_constant)) {
>
> +      PP.Diag(Tok.getLocation(), diag::err_expected) <<
>
> +      "enable, disable or integer value";
>
> +      return;
>
> +    }
>
> +
>
> +    Info->Value = Tok;
>
> and:
>
> +  if (Info->Value.is(tok::numeric_constant))
>
> +    Hint.ValueExpr = Actions.ActOnNumericConstant(Info->Value);
>
> looks wrong. We should parse arbitrary constant expressions here. The
> OpenMP code for simdlen, etc. does this, please emulate that.
>
> Thanks again!
>
>  -Hal
>
> >
> >
> > Thanks,
> >
> >
> > Tyler
> >
> >
> > On Apr 28, 2014, at 4:57 PM, Hal Finkel < [email protected] > wrote:
> >
> >
> >
> > ----- Original Message -----
> >
> >
> > From: "Tyler Nowicki" < [email protected] >
> > To: "Alexander Musman" < [email protected] >
> > Cc: "Alexey Bataev" < [email protected] >, "llvm cfe" <
> > [email protected] >, "Nadav Rotem" < [email protected] >
> > Sent: Monday, April 28, 2014 6:53:40 PM
> > Subject: Re: [PATCH] #pragma vectorize
> >
> >
> >
> > Hi Alexander,
> >
> > Thanks for the review. I’ve updated the patch with your most of your
> > suggestions.
> >
> > Please review the updated patch.
> >
> >
> >
> > Do you plan to support substitution of integer template parameters
> > into pragma? Here is example:
> >
> >
> > cat ploop.cpp
> > template <int VLEN> void while_test(int *List, int Length) {
> > int i = 0;
> > #pragma loop vectorize(VLEN)
> > while(i < Length) {
> > List[i] = i*2;
> > i++;
> > }
> > }
> > int main() {
> > int L[100];
> > while_test<4> (L, 100);
> > return 0;
> > }
> >
> > I considered this, it is not currently supported. I’m not sure if it
> > makes sense. Lets leave this as a topic of future work for now.
> >
> > This kind of use case *must* be supported, it is very important for
> > libraries that auto-tune. Is there some reason that it is
> > complicated to support?
> >
> > -Hal
> >
> >
> >
> >
> >
> >
> >
> > ParsePragma.cpp:1619: // Read '('
> > This looks like a good place to use BalancedDelimiterTracker for
> > parsing '(' and ')’.
> >
> > I don’t think it is needed. It isn’t used by any other #pragma
> > directives and the syntax here is rather simple. What do you think
> > would be the benefit?
> >
> > Tyler
> >
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > [email protected]
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> >
> >
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> >
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to