<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
I'm hesitant to add a new analysis pass because keeping the analysis up-to-date is inevitably painful... especially given that GVN probably wants to check this at higher resolution than just per-block.  </blockquote><div><br></div><div>Again, it's very wasteful for it to have to know/compute (even once), where the blocks that it has to look at in more detail are, when nobody makes things *more* maythrow then they were before.</div><div><br></div><div>If the CFG properly represented "this block, somewhere, throws", it would know "okay, if i want to hoist out of this block, i have to look harder".</div><div><br></div><div>Instead, we now have a bunch of passes that are, at a minimum, going to look at every instruction in each block and start keeping track of the same info. In fact, every patch we've done so far here (even mine!) tries to recompute this very info so it can figure out where it needs to look harder in constant time.  </div><div><br></div><div>Also, the current PRE does not really do good PRE, and makes no attempt to place instructions like a real PRE would.</div><div>Any real PRE or store sinking pass is going to want to know what blocks are "transparent", that is, it is possible to hoist or sink through.</div><div><br></div><div>MemorySSA will tell them if the memory state is killed in the block in constant time.</div><div>SSA will tell them if the operands are killed or reusable in the block in constant time.</div><div><br></div><div>The per-block bit you are talking about is precisely the last piece that tells you if a block is transparent in constant time.</div><div><br></div><div>Without it, you must touch every instruction in the program to know which blocks are transparent.</div><div><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Maybe it's the right approach, though.<br>
<span class=""><br>
> At minimum, the patch should be rewritten as an extension to isValueFullyAvailableInBlock; that's the bit that's supposed to be reasoning about anticipation for LoadPRE.<br>
<br>
<br>
</span>I'm not following; this isn't something we need to check on a per-predecessor basis.  The issue is basically whether it's legal to hoist a given load from the middle of its parent BB to the beginning of that BB.<br></blockquote><div><br></div><div>This is just because of how current PRE works (which, as mentioned, is pretty wasteful). If you only want to fix that behavior, then yeah, compute it once, use that.</div><div><br></div></div></div></div>