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:[email protected]]
*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
<[email protected]<mailto:[email protected]>> 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
[email protected]<mailto:[email protected]>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
