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

[Andrew Trick]

On Mar 5, 2012, at 12:27 AM, Duncan Sands <baldrick at free.fr> wrote:

>> Sure, it all depends on reading "via" as including the end point or not :-)
> 
> Yeah, that's dominates vs properly dominates.  The basic block stuff makes
> this distinction already.  The instruction stuff doesn't, presumably because
> only proper domination is useful; and I'm guessing it's named "dominates"
> instead of "properlyDominates" for instructions because it is shorter to
> write.  I don't suggest changing the name: a comment would be enough I reckon.

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.

-Andy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120305/1dd1671b/attachment.html>