[PATCH] #pragma vectorize

[Alexey Bataev]

Hi everybody, my comments.

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.
2. Are you going  to support this pragma for range-based for loop from 
C++11 (CXXForRangeStmt in AST)?
3. +  if (Tok.is(tok::annot_pragma_loop_hint) ||
+      Tok.is(tok::kw_while) || Tok.is(tok::kw_do) || Tok.is(tok::kw_for)) {
+    StmtResult NextStmt = ParseStatement();
+
+    if (!NextStmt.isUsable())
+      return NextStmt;
+
+    Stmt *Loop = NextStmt.get();
+    if (isa<AttributedStmt>(Loop))
+      Loop = cast<AttributedStmt>(Loop)->getSubStmt();

This code does not allow using of attributed statements because it 
expects only 'for', 'do' or 'while' right after #pragma.

4. There is no serialization/deserialization support for LoopHints. Also 
add a test for it to verify that it works properly.
5. Is it ok that currently pragma allows using only of literal constants 
in it clauses, but not constant expressions?
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)?
7. No support for -ast-dump and -ast-print options.

I strongly recommend to ask Doug Gregor, Richard Smith to review this code.

-- 
Best regards,
Alexey Bataev
=============
Software Engineer
Intel Compiler Team
Intel Corp.



*From:*Tyler Nowicki [mailto:tnowicki at apple.com]
*Sent:* Wednesday, April 23, 2014 3:54 AM
*To:* Hal Finkel; Chandler Carruth
*Cc:* Bataev, Alexey; Nadav Rotem; llvm cfe
*Subject:* Re: [PATCH] #pragma vectorize

Please review this updated patch. It includes the changes we discussed. 
Thanks for all your input!

Tyler

On Apr 22, 2014, at 2:30 PM, Tyler Nowicki 
<tnowicki at apple.com<mailto:tnowicki at apple.com>> wrote:



Hi Hal,

Thanks for the reply.

Maybe we're looking at this the wrong way... what about?

pragma loop vectorize(width/enable/disable) interleave(count/enable/disable)

I like this more, especially because its clear it applies only to loops.





enable/disable don’t add
anything that isn’t already part of pragma vectorize enable/disable,
and specifying `#pragma vectorize disable’ would disable
interleaving.


But that's a bug. Are you sure that's what happens?

I could be mistaken. This is what is in LoopVectorize at the top of 
processLoop()

     if (Hints.Force == 0) {

       DEBUG(dbgs() << "LV: Not vectorizing: #pragma vectorize disable.\n");

       return false;

     }

And the unrolling occurs later in processLoop(). I thought it was a 
feature… but yea, lets fix it.







As for safety, how about #pragma vectorize aggressive?


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.

`nodependence’?

Thanks,

Tyler

_______________________________________________
cfe-commits mailing list
cfe-commits at cs.uiuc.edu<mailto:cfe-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140423/676a7b65/attachment.html>