----- 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]>, "Douglas Gregor" > <[email protected]> > 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 < [email protected] > wrote: > > > > ----- 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 > -- 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
