<div dir="ltr">So i have a testcase (see PR31792, and cond_br2.llin GVN) that current GVN can simplify because it replaces instructions as it goes.  It's an example of a larger issue that pops up quite a lot<div>I would appreciate thoughts on what to do about it</div><div>it amounts to something like this (but again, it happens a lot):<br><br></div><div>live = gep thing, 0</div><div>live2 = gep thing, 1</div><div>branch i1 provablytrue,, mergeblock, otherbb</div><div>otherbb:<br>dead = something else<br></div><div>br mergeblock</div><div>merge block</div><div>a = phi(live, dead)</div><div>b = live2</div><div>result = icmp sge a, b</div><div><br></div><div>both GVN and NewGVN prove provablytrue to be true, and phi to be equivalent to live.</div><div><br></div><div>GVN transforms this piece at time, and so by the time simplifycmpinst sees the icmp, it is</div><div><br></div><div>result = icmp sge <live2, live></div><div><br></div><div>It proves result true.</div><div><br></div><div>NewGVN is an analysis, and so it doesn't transform the ir, and simplifycmpinst (rightfully) does not try to walk everything, everywhere, to prove something. It also couldn't know that dead is dead. So it doesn't see that result is true.</div><div><br></div><div>The upshot is that it takes two passes of newgvn to get the same result as gvn.</div><div><br></div><div>I'm trying to decide what to about this case. As i said, it happens a lot.</div><div><br>It would be pretty trivial to make a "VNImpl" interface that has a few functions (that can be fulfilled by any value numbering that is an analysis), have newgvn implement it, and use it in Simplify*.</div><div><br></div><div>(It would take work to make GVN work here because of how many places it modifies the ir during value numbering. It also modifies as it goes, so the only advantage would be from unreachable blocks it discovers)</div><div><br></div><div>But before i add another argument to functions taking a ton already[1], i wanted to ask whether anyone had any opinion on whether it's worth doing.</div><div><br></div><div>VNImpl would probably look something like:<br>class VNImpl{</div><div>// return true if A and B are equivalent</div><div>bool areEquivalent(Value *A, Value *B);</div><div>// find a value that dominates A that is equivalent to it</div><div>Value *findDominatingEquivalent(Value *A);</div><div>// obvious</div><div>bool isBlockReachable(BasicBock *BB);</div><div>}</div><div><br></div><div><br></div><div>Thoughts on whether to do this (or alternatives), appreciated.</div><div><br></div><div>[1]  I'm also unsure if i should do something about the number of arguments these functions take, which would be 9 or 10 in a lot of cases now, since some take 8 or 9. It seems like it may be time to just have a context structure that provides all the optional data you want here.</div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div></div>