[PATCH] [OPENMP] llvm.loop.vectorize.enable metadata are lost after critical edge splitting

Johannes Doerfert doerfert at cs.uni-saarland.de
Tue Sep 30 15:57:14 PDT 2014


>>! In D5539#16, @hfinkel wrote:
> ----- Original Message -----
>> From: "Andrew Trick" <atrick at apple.com>
>> To: "zinovy nis" <zinovy.nis at gmail.com>, chandlerc at gmail.com, atrick at apple.com
>> Cc: llvm-commits at cs.uiuc.edu
>> Sent: Tuesday, September 30, 2014 4:29:08 PM
>> Subject: Re: [PATCH] [OPENMP] llvm.loop.vectorize.enable metadata are lost	after critical edge splitting
>> 
>> Is it incorrect to leave the loop metadata on the original branch
>> when splitting the edge?
>> 
>> GVN already requires DomTree, so no change to the pipeline is needed
>> if you use it.
>> 
>> In splitCriticalEdges, if LoopInfo is available you can use it.
>> Otherwise you can use the DomTree. If the DomTree check fails (ie.
>> both targets dominate the latch) then just invalidate the metadata.
>> 
>> I think there is a fundamental problem with the metadata design--it's
>> ridiculous to assume that any pass that splits critical edges has
>> LoopInfo--but that should at least workaround your problem.
> 
> Is that a flaw in the design of the metadata, or just in the fact that the utility functions used to manipulate the metadata are part of LoopInfo?
> 
>  -Hal
In my opininion the real probelem is the static SplitCriticalEdge function which is called with a black box Pass * instead of the two analysis passes it could actually use to update information.
It is intransparent to the user what passes should be provided to ensure the best result.

The "utility functions to manipulate the metadata" are actually only available in this and the other pending patch http://reviews.llvm.org/D5344 and both currently use LoopInfo. However, we could update them to handle metadata also with the DomTree. In any case the other patch already invalidates metadata if no analysis is present.

http://reviews.llvm.org/D5539






More information about the llvm-commits mailing list