[PATCH] MemoryDependenceAnalysis always depends on DominatorTree

[Nick Lewycky]

On 5 April 2013 15:22, Matt Arsenault <Matthew.Arsenault at amd.com> wrote:

>  On 04/04/2013 01:04 PM, Nick Lewycky wrote:
>
> No. The test for DT->isReachableFromEntry(I->getBB()) is to ensure that
> the code that follows it is safe. Just because you don't have a DT
> available doesn't mean that the code in the if-body is safe.
>
>  I don't understand. My interpretation of Bill's comment is that this
> should work correctly (a conservative answer for the DT check would be that
> a block is reachable from isReachableFromEntry).
>

I think the conservative answer goes the other way, that the block can not
be assumed to be reachable. Here's what's going on:

This function is not valid IR:

  define i32 @test() {
  entry:
    %a = add i32 %a, 1
    ret i32 %a
  }

but this function is:

  define i32 @test() {
  entry:
    ret i32 0
  unreachable_block:
    %a = add i32 %a, 1
    ret i32 %a
  }

Memdep wants to start at the bottom of a use-def chain and work its way
upwards. The problem is that one of the values feeding it may actually come
from dead code (through a PHI node). If you reach dead code, then you can
encounter IR like the above and visit %a and visit %a's operands which
include %a itself which leads to an infinite loop.

It's not the common case, but it happens. It happens rarely enough that
this bug was in memdep for a long time before Bill fixed it. There are two
ways that LLVM deals with this: either ask the DominatorTree whether the
block is reachable from the entry block (and therefore traversing from
definition to operands will never cause an infinite loop) or keeping track
of visited instructions in a SmallSet.

So if you don't know whether a block is reachable from entry, you have to
assume it isn't safe to traverse from definition to operands.

You could also try implementing an amalgam and having a SmallSet that you
never use unless DT is null.

Nick
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130405/2fc8deec/attachment.html>