[llvm-commits] [llvm] r151466 - in /llvm/trunk: include/llvm/Analysis/Dominators.h lib/Analysis/InstructionSimplify.cpp

[Duncan Sands]

Hi Chris,

On 02/03/12 22:18, Chris Lattner wrote:
>
> On Mar 2, 2012, at 12:44 PM, Duncan Sands wrote:
>
>> Hi Rafael,
>>
>>> Don't call dominates on unreachable instructions.
>>
>> why not?  Is there really no sensible definition of domination in this case?
>
> There is no good answer.  The definition of dom(A,B) is "all paths from the entry block to B go through A".  This is vacuously true for unreachable blocks, but it is unlikely that any transformation would be prepared to handle this (because now you can have dom(a,b) = true and dom(b,a) = true and a != b).

all kinds of transformations already have to handle this, in fact you hit an
example yourself not long ago: when removing bitcasts from an instruction in
an unreachable block, it is possible that after peeling of a number of bitcasts
you get back to the original instruction!

>
>> But perhaps you have a different definition of domination in mind?
>
> I think that we're all on the same page, but that doesn't necessarily make it a useful API for the dominators class.
>
>> PS: If you add an assertion about dominates(instruction,instruction) only
>> being called for reachable instructions, then shouldn't you have one for
>> dominates(basic block, basic block) too?  After all, if you think
>> dominates(instruction,instruction) has no sensible meaning, probably
>> dominates(basic block, basic block) has no sensible meaning either.
>
> Yes, I think it is worth being consistent here.  For blocks, we return false in this case iirc, which would be better than returning true.

See for example the implementation of isReachableFromEntry.  If some value for
"dominates" with unreachable basic blocks falls naturally out of the dominators
computation, maybe whatever that is (is it really just "false" always?) can be
extended to dominates(instr,instr).

Ciao, Duncan.