<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div></div><br><div><div>On Apr 22, 2014, at 11:25 PM, Alexey Bataev <<a href="mailto:a.bataev@hotmail.com">a.bataev@hotmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div text="#000000" bgcolor="#FFFFFF" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="moz-cite-prefix">Another one thing.<br>It seems to me that currently this implementation does not support template instantiation. Need a test for it.<br><pre class="moz-signature" cols="72">Best regards,
Alexey Bataev
=============
Software Engineer
Intel Compiler Team
Intel Corp. </pre>23.04.2014 7:43, Alexey Bataev пишет:<br></div><blockquote cite="mid:BLU0-SMTP12315C1D998A7A3D0BEE5E885580@phx.gbl" type="cite">Hi everybody, my comments.<br><br>1. Agree with Chandler about multiple inheritance. I think it would be better to create class LoopWithHintsStmt inherited from Stmt and make it a base class for all loops.<br>2. Are you going to support this pragma for range-based for loop from C++11 (CXXForRangeStmt in AST)?<br>3. + if (<a href="http://Tok.is">Tok.is</a>(tok::annot_pragma_loop_hint) ||<br>+ <a href="http://Tok.is">Tok.is</a>(tok::kw_while) || <a href="http://Tok.is">Tok.is</a>(tok::kw_do) || <a href="http://Tok.is">Tok.is</a>(tok::kw_for)) {<br>+ StmtResult NextStmt = ParseStatement();<br>+<br>+ if (!NextStmt.isUsable())<br>+ return NextStmt;<br>+<br>+ Stmt *Loop = NextStmt.get();<br>+ if (isa<AttributedStmt>(Loop))<br>+ Loop = cast<AttributedStmt>(Loop)->getSubStmt();<br><br>This code does not allow using of attributed statements because it expects only 'for', 'do' or 'while' right after #pragma.<br><br>4. There is no serialization/deserialization support for LoopHints. Also add a test for it to verify that it works properly.<br>5. Is it ok that currently pragma allows using only of literal constants in it clauses, but not constant expressions?<br>6. What if I use several incompatible clauses for the same loop? For example, how it is supposed to work if I have #pragma vectorize(enable) vectorize(disable)?<br>7. No support for -ast-dump and -ast-print options.<br><br>I strongly recommend to ask Doug Gregor, Richard Smith to review this code.<br><pre class="moz-signature" cols="72">--
Best regards,
Alexey Bataev
=============
Software Engineer
Intel Compiler Team
Intel Corp.
</pre><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><a moz-do-not-send="true" name="_MailOriginal"><b><span lang="EN-US" style="font-size: 10pt; font-family: Tahoma, sans-serif;">From:</span></b></a><span><span lang="EN-US" style="font-size: 10pt; font-family: Tahoma, sans-serif;"><span class="Apple-converted-space"> </span>Tyler Nowicki [<a moz-do-not-send="true" class="moz-txt-link-freetext" href="mailto:tnowicki@apple.com" style="color: purple; text-decoration: underline;">mailto:tnowicki@apple.com</a>]<span class="Apple-converted-space"> </span><br><b>Sent:</b><span class="Apple-converted-space"> </span>Wednesday, April 23, 2014 3:54 AM<br><b>To:</b><span class="Apple-converted-space"> </span>Hal Finkel; Chandler Carruth<br><b>Cc:</b><span class="Apple-converted-space"> </span>Bataev, Alexey; Nadav Rotem; llvm cfe<br><b>Subject:</b><span class="Apple-converted-space"> </span>Re: [PATCH] #pragma vectorize<o:p></o:p></span></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>Please review this updated patch. It includes the changes we discussed. Thanks for all your input!<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>Tyler<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>On Apr 22, 2014, at 2:30 PM, Tyler Nowicki <</span><a moz-do-not-send="true" href="mailto:tnowicki@apple.com" style="color: purple; text-decoration: underline;"><span><span>tnowicki@apple.com</span></span><span></span></a><span>> wrote:<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><br><br><o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>Hi Hal,<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>Thanks for the reply.<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>Maybe we're looking at this the wrong way... what about?<br><br>pragma loop vectorize(width/enable/disable) interleave(count/enable/disable)<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>I like this more, especially because its clear it applies only to loops.<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><br><br><o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><br><br><o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>enable/disable don’t add<br>anything that isn’t already part of pragma vectorize enable/disable,<br>and specifying `#pragma vectorize disable’ would disable<br>interleaving.<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><br>But that's a bug. Are you sure that's what happens?<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>I could be mistaken. This is what is in </span><span><span style="font-size: 8.5pt; font-family: Menlo, serif;">LoopVectorize at the top of processLoop()</span></span><span><span><o:p></o:p></span></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><span style="font-size: 8.5pt; font-family: Menlo, serif;"> if (Hints.Force == 0) {<o:p></o:p></span></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><span style="font-size: 8.5pt; font-family: Menlo, serif;"> <span class="Apple-converted-space"> </span>DEBUG(dbgs() << "LV: Not vectorizing: #pragma vectorize disable.\n");<o:p></o:p></span></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><span style="font-size: 8.5pt; font-family: Menlo, serif;"> <span class="Apple-converted-space"> </span>return false;<o:p></o:p></span></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><span style="font-size: 8.5pt; font-family: Menlo, serif;"> <span class="Apple-converted-space"> </span>}<o:p></o:p></span></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><span style="font-size: 8.5pt; font-family: Menlo, serif;"> </span></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>And the unrolling occurs later in processLoop(). I thought it was a feature… but yea, lets fix it.<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><br><br><o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><br><br><o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><br><br>As for safety, how about #pragma vectorize aggressive?<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span><br>I don't like that; *that* sounds like a cost-model adjustment. The user is asserting something about the property of the loop, and we should try to capture that property. Although this may just be confusing, "vectorizable" is what we mean.<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>`nodependence’? <o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>Thanks,<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>Tyler<o:p></o:p></span></div><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span>_______________________________________________<br>cfe-commits mailing list<br></span><a moz-do-not-send="true" href="mailto:cfe-commits@cs.uiuc.edu" style="color: purple; text-decoration: underline;"><span><span>cfe-commits@cs.uiuc.edu</span></span><span></span></a><span><span><br></span></span><a moz-do-not-send="true" href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" style="color: purple; text-decoration: underline;"><span><span>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</span></span><span></span></a><span><span><o:p></o:p></span></span></div><span></span><div style="margin: 0cm 0cm 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span> </span></div><pre class="moz-signature" cols="72"></pre></blockquote><br></div><br class="Apple-interchange-newline"></blockquote></div><br></body></html>