[PATCH] #pragma vectorize

Hal Finkel hfinkel at anl.gov
Tue Apr 29 11:24:16 PDT 2014


----- Original Message -----
> From: "Tyler Nowicki" <tnowicki at apple.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Alexey Bataev" <a.bataev at hotmail.com>, "llvm cfe" <cfe-commits at cs.uiuc.edu>, "Nadav Rotem" <nrotem at apple.com>,
> "Alexander Musman" <alexander.musman at gmail.com>, "Douglas Gregor" <dgregor at apple.com>
> Sent: Tuesday, April 29, 2014 12:50:52 PM
> Subject: Re: [PATCH] #pragma vectorize
> 
> 
> I think its a little more complicated than that unfortunately. The
> pragma does not contain annotated template tokens. Those tokens
> don't get generated for pragmas in general.

What do you mean?

> 
> 
> I’m not saying we shouldn’t do this. I just think we should leave it
> for another commit.

I'm fine with this as incremental progress, so long as the follow-up happens in the near term. Please add a FIXME describing what needs to change to support constant expressions (including use via template instantiation).

 -Hal

> 
> 
> Tyler
> 
> 
> On Apr 28, 2014, at 5:19 PM, Hal Finkel < hfinkel at anl.gov > wrote:
> 
> 
> 
> ----- Original Message -----
> 
> 
> From: "Tyler Nowicki" < tnowicki at apple.com >
> To: "Hal Finkel" < hfinkel at anl.gov >
> Cc: "Alexey Bataev" < a.bataev at hotmail.com >, "llvm cfe" <
> cfe-commits at cs.uiuc.edu >, "Nadav Rotem" < nrotem at apple.com >,
> "Alexander Musman" < alexander.musman at gmail.com >
> 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 < hfinkel at anl.gov > wrote:
> 
> 
> 
> ----- Original Message -----
> 
> 
> From: "Tyler Nowicki" < tnowicki at apple.com >
> To: "Alexander Musman" < alexander.musman at gmail.com >
> Cc: "Alexey Bataev" < a.bataev at hotmail.com >, "llvm cfe" <
> cfe-commits at cs.uiuc.edu >, "Nadav Rotem" < nrotem at apple.com >
> 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
> cfe-commits at cs.uiuc.edu
> 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
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory




More information about the cfe-commits mailing list