llvm dev - Jul 2017 - CGP: Break use-def graph loops in optimizeMemoryInst

Dear Community,

I'm trying to implement optimization described in PR26223 and meet the
following bail out condition in CodeGenPrepare::optimizeMemoryInst.

    // Break use-def graph loops.
    if (!Visited.insert(V).second) {
      Consensus = nullptr;
      break;
    }

So while traversing thorough phi nodes from memory instruction to find addr mode
we bail out if we meet some instruction twice.
Does anybody know/remember what is the reason for this
checks.
>From the first glance it seems that it will be ok if we just mark it as
visited and continue processing worklist.
I did this change and make check passed.

So I seek for a help to explain this decision.

Thank you in advance,
Serguei.

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170630/922c2573/attachment.html>

My speculation here is that we originally didn't have the check for 
cycles and someone (maybe even the original author) added it to prevent 
stack overflow due to infinite recursion.  I do not see a strong reason 
not to handle phi cycles in this code.  Curious if anyone else 
disagrees?  Or was around for the design of this?

Philip

On 06/29/2017 08:47 PM, Serguei Katkov via llvm-dev
wrote:
>
> Dear Community,
>
> I'm trying to implement optimization described in PR26223 and meet the 
> following bail out condition in CodeGenPrepare::optimizeMemoryInst.
>
>     // Break use-def graph loops.
>
>     if (!Visited.insert(V).second) {
>
>       Consensus = nullptr;
>
>       break;
>
>     }
>
> So while traversing thorough phi nodes from memory instruction to find 
> addr mode we bail out if we meet some instruction twice.
>
> Does anybody know/remember what is the reason for this checks.
>
> From the first glance it seems that it will be ok if we just mark it 
> as visited and continue processing worklist.
>
> I did this change and make check passed.
>
> So I seek for a help to explain this decision.
>
> Thank you in advance,
>
> Serguei.
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170701/421a2be0/attachment.html>