<div dir="ltr"><div class="im" style="font-family:arial,sans-serif;font-size:12.800000190734863px"><div class="im">>>><br>>>> ParsePragma.cpp:1619: // Read '('<br>>>> This looks like a good place to use BalancedDelimiterTracker for parsing '(' and ')’.<br>
<br></div><span style="color:rgb(34,34,34)">>>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?</span><br></div><div class="im" style="font-family:arial,sans-serif;font-size:12.800000190734863px">
<span style="color:rgb(34,34,34)"><br></span></div><div class="im" style="font-family:arial,sans-serif;font-size:12.800000190734863px"><font color="#222222">This would improve consistency with the other places in clang where '(' and ')' are parsed. Otherwise it seems to be equivalent to handling '(' and ')' manually.</font></div>
<div class="im" style="font-family:arial,sans-serif;font-size:12.800000190734863px"><font color="#222222"><br></font></div><div class="im" style="font-family:arial,sans-serif;font-size:12.800000190734863px"><font color="#222222"><br>
</font></div><div class="im" style="font-family:arial,sans-serif;font-size:12.800000190734863px"><div class="im">>>> It would be very useful for auto-tuning libraries certainly. I am not<br>>>> sure how to support it currently. I suggest we leave it for a future<br>
>>> commit.<br><br></div><span style="color:rgb(34,34,34)">>>I suspect that Alexey and Alexander can provide advice here.</span><br></div><div class="im" style="font-family:arial,sans-serif;font-size:12.800000190734863px">
<span style="color:rgb(34,34,34)"><br></span></div><div class="im" style><font color="#222222" style="font-family:arial,sans-serif;font-size:12.800000190734863px">For openmp clauses, we have methods in TreeTransform for this (e.g. </font><font color="#222222" face="arial, sans-serif">TransformOMPSafelenClause). You will probably need something similar for attributes.</font></div>
<div class="im" style="font-family:arial,sans-serif;font-size:12.800000190734863px"><span style="color:rgb(34,34,34)"><br></span></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-04-29 4:19 GMT+04:00 Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">----- Original Message -----<br>
> From: "Tyler Nowicki" <<a href="mailto:tnowicki@apple.com">tnowicki@apple.com</a>><br>
</div><div class="">> To: "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>><br>
> Cc: "Alexey Bataev" <<a href="mailto:a.bataev@hotmail.com">a.bataev@hotmail.com</a>>, "llvm cfe" <<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a>>, "Nadav Rotem" <<a href="mailto:nrotem@apple.com">nrotem@apple.com</a>>,<br>

> "Alexander Musman" <<a href="mailto:alexander.musman@gmail.com">alexander.musman@gmail.com</a>><br>
> Sent: Monday, April 28, 2014 7:09:13 PM<br>
> Subject: Re: [PATCH] #pragma vectorize<br>
><br>
><br>
</div><div class="">> It would be very useful for auto-tuning libraries certainly. I am not<br>
> sure how to support it currently. I suggest we leave it for a future<br>
> commit.<br>
<br>
</div>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.<br>

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