[PATCH] D74931: [Dominators] Use Instruction::comesBefore for block-local queries, NFC

[Vedant Kumar via Phabricator via llvm-commits]

vsk marked an inline comment as done.
vsk added inline comments.


================
Comment at: llvm/lib/IR/Dominators.cpp:284
   // everything in the block.
   if (isa<PHINode>(UserInst))
     return true;
----------------
kuhar wrote:
> This code makes every user that is a phi be dominated by the def?
> So if we have two phis, the one that comes later may be said to dominate its predecessor?
> 
> Not sure if I got confused here, but this looks fishy to me. Do we need this if at all?
Yes, that's what this is doing, and no, I'm not sure this is /really/ needed.

IIUC all phis in a block are thought to execute "instantly, together". The verifier tries to make this clear by enforcing grouping of phis at the start of a block. Perhaps this is part of the enforcement mechanism, by making `phi1 dom phi2` = `phi2 dom phi1`?

I find this inherently confusing, fwiw it's mentioned as a motivation for basic block arguments in the SIL/MLIR docs.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74931/new/

https://reviews.llvm.org/D74931