[PATCH] #pragma vectorize
Tyler Nowicki
tnowicki at apple.com
Sun Apr 27 17:00:22 PDT 2014
Hi,
Thanks to everyone for all their feedback. Please review the updated patch.
I modified the implementation to use attributed statements on the loop stmts to pass loop hints to the code generator. The benefit of using this method is that c++11 loop attributes won’t take too much more work to support, such as defining the syntax. Also attributed statements are part of the AST, along with their data.
The syntax remains unchanged as:
#pragma loop vectorize(enable/disable/value) interleave(enable/disable/value)
In response to Alexey Bataev:
1. Multiple-inheritance: Fixed using attributes to pass loop hints instead.
2. Support range-based loop: Done.
3. Loop Attributes: In theory this should work, but its untested. I couldn’t find any attributes that apply to loops or statements in general.
4. Serialization/deserialization: Should be supported with attributed stmts.
5. Expressions: Fixed. The value is now interpreted as a constant integer expression.
6. Contradictory directives: I don’t think this is a problem because there are only really two options vectorize and interleave. Contradictions could be easily checked if you think it would be a problem.
7. ast-dump and ast-print: Should be supported on attributed stmts.
8. Templates: Done and tests have been added.
In response to Hal Finkel:
- I made the changes you suggested. If I missed anything please let me know.
- I think the documentation should be added in an separate commit. I’ll get started on that next.
- I don’t know Doug or Richard, could you add them to the cc-list.
Looking forward to more feedback,
Tyler
On Apr 22, 2014, at 11:25 PM, Alexey Bataev <a.bataev at hotmail.com> wrote:
> Another one thing.
> It seems to me that currently this implementation does not support template instantiation. Need a test for it.
> Best regards,
> Alexey Bataev
> =============
> Software Engineer
> Intel Compiler Team
> Intel Corp.
> 23.04.2014 7:43, 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> 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
>> 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/20140427/1a2a3b97/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pragma_loop-svn.patch
Type: application/octet-stream
Size: 32306 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140427/1a2a3b97/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140427/1a2a3b97/attachment-0001.html>
More information about the cfe-commits
mailing list