<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, May 27, 2014 at 12:04 PM, Tyler Nowicki <span dir="ltr"><<a href="mailto:tnowicki@apple.com" target="_blank">tnowicki@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>Hi Richard,</div><div><br></div><div>Thanks for the review.</div><div><br></div>
<div>You’ve suggested copying HandlePragmaMSPragma, however, the behavior is already nearly identical. My own pragma parsing does some up-front error checking and then puts the relevant tokens into a structure that is attached to an annotated token. MSPragma does no up-front error checking and puts all tokens into a list that is attached to an annotated token. During parsing of the loop hint annotated token sema (ActOnLoopHintValue) handles all cases for the loop hint argument. During parsing of the MSPragma annotated token some delegates handle each flavor of mspragma and make calls to sema. If there is something I’ve missed here please let me know what that is.</div>
</div></blockquote><div><br></div><div>Right, yes, you're on the right lines. What I'm suggesting is that you grab the token sequence between the parentheses and store that until you get to parsing the annotation. Then put them back into the token stream and use the normal expression parsing machinery to parse the expression. That's the part of HandlePragmaMSPragma's behavior that I was referring to. (I can see how that was unclear, sorry!)</div>
<div><br></div><div>Once you've done that, you shouldn't need any custom template instantiation logic.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div>We’ve invented this pragma. We have also already had a long discussion on its syntax in the `#pragma vectorize’ thread. The consensus was for the pragma to take ‘enable’, ‘disable’, and a non-type template argument. I don’t see why adding constant expressions should be a problem. I will do this in a follow-up commit.</div>
<div><br></div><div>If you have any suggestions for an alternate syntax please let us know. We would be very interested in hearing your suggestions.</div></div></blockquote><div><br></div><div>Off the top of my head:</div>
<div><br></div><div>#pragma clang loop vectorize(enable, <n>)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">
<div>Also, would you please review the patch in the `#pragma vectorize’ thread. This is my third ping.</div></div></blockquote><div><br></div><div>Sorry about the delay on that thread!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div>Thanks,</div><div><br></div><div>Tyler</div><div><div class="h5"><br><div><div>On May 26, 2014, at 9:55 AM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:</div>
<br><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, May 23, 2014 at 2:41 PM, Tyler Nowicki <span dir="ltr"><<a href="mailto:tnowicki@apple.com" target="_blank">tnowicki@apple.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi,<br>
<br>
As requested here is the patch to support non-type template parameters in #pragma loop. The patch uses LookupName to build a DeclRefExpr to the template parameter and overloads TransformAttr in the template instantiator to transform the value expression and replace the LoopHintAttr. It is built on the PragmaAttr and Pragma Spelling patch listed in the 'Add PragmaAttr and Pragma Spelling to Tablegen’ thread. Since it is a git patch please use ‘patch -p1’ to apply it.<br>

</blockquote><div><br></div><div>This seems like fundamentally the wrong approach to me. It doesn't make much sense to me to accept only an identifier here; at a minimum you should accept an id-expression, and ideally you should accept any constant-expression. And that in turn means your approach of parsing the pragma from the lexer is unworkable -- you need to move the parsing logic to the parser in order to be able to actually parse an id-expression / constant-expression. See HandlePragmaMSPragma for an example of an existing pragma that does this.</div>

<div><br></div><div>Is there some pre-existing standard / specification we're implementing here, or is this a pragma of our own invention?</div><div><br></div><div>Also:</div><div><br></div><div><div>+  if (ValueName == "enable" || ValueName == "disable")</div>

<div>+    return ExprEmpty();</div></div><div><br></div><div>If this is a pragma we invented, can we design it such that we don't have an ambiguity between its pseudo-keywords and its identifier lookup?</div></div></div>

</div>
</blockquote></div><br></div></div></div></blockquote></div><br></div></div>