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

[Andrew Trick]

On Mar 5, 2012, at 12:36 PM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:

>> The original dominates(Inst, Inst) did not implement proper dominance. I
>> added a properlyDominates(Inst, Inst) for that purpose. Rafael later
>> reasoned that all clients of dominates(Inst, Inst) actually want proper
>> dominance and changed the implementation accordingly--I tend to agree
>> because I've seen multiple bugs based on that assumption. I suggested
>> removing properlyDominates(Inst, Inst), because it's misleading if it
>> actually does the same thing as dominates(Inst, Inst). Given that it only
>> makes sense to implement instruction-level queries as proper dominance (def
>> dominates use is the implied semantics), it shouldn't matter what the name
>> is, as long as the comments are clear.
> 
> Thanks for the comments! I think the summary of the thread so far is
> 
> * Lets keep the current names, just make it clear that dominates(inst,
> inst) is not exactly the same as dominates(bb,bb).
> * Dominates for instructions stays as is.
> * Dominates for BB is changed so that anything dominates an
> unreachable block. This does create a case where
>  A != B && dominates(A, B) && dominates(B,A) is true.

So, we can have dominates(I1, I1) = = true, if I1 is an instruction (or phi) in an unreachable block, right?

> I still have a small preference for renaming one of the functions or
> making them behave more like one another, but I agree that this would
> be a bit better than what we have now. Chris, would you be OK with a
> patch implementing it?

"dominatesUse()" was ok, but as Duncan pointed out could be misleading, since there's no assumption of data flow between operands. "properlyDominates()" could be misleading because of the unreachable case above, and theoretically wrong because "proper" dominance essentially refers nodes in the domtree, not instructions. I just don't have a better idea.

-Andy