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

[Andrew Trick]

> On Sep 30, 2014, at 2:37 PM, Hal Finkel <hfinkel at anl.gov> 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?

I haven’t reviewed the metadata design since it was put in place. Based on this patch, the problem is that splitCriticalEdges needs to update the metadata (that is itself a somewhat bad design), and updating the metadata requires LoopInfo to determine whether we’re splitting the loop backedge or not. My suggestion is simply to use the DomTree to determine whether we’re splitting the backedge and update the metadata without requiring LoopInfo.

You’re correct that my approach cannot use the setLoopID() utility, but I think that’s only part of the problem.

-Andy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140930/8a8ea85d/attachment.html>