<div dir="ltr">Yes.<div>If it was an Analysis, it could be preserved.</div><div>As a utility, it isn't.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 29, 2017 at 1:43 PM, Xin Tong <span dir="ltr"><<a href="mailto:trent.xin.tong@gmail.com" target="_blank">trent.xin.tong@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Because analysis might be used by later passes ?<br>
-Xin<br>
<div class="HOEnZb"><div class="h5"><br>
On Fri, Jun 30, 2017 at 5:35 AM, Daniel Berlin <<a href="mailto:dberlin@dberlin.org">dberlin@dberlin.org</a>> wrote:<br>
> I don't think we should make utilities use AssertingVH like this, at least<br>
> by default.  Utilities are often built and thrown away.  If it was an<br>
> analysis, I'd completely agree we should use AssertingVH<br>
><br>
><br>
> On Thu, Jun 29, 2017, 1:33 PM Xin Tong via Phabricator<br>
> <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br>
>><br>
>> trentxintong added a comment.<br>
>><br>
>> In <a href="https://reviews.llvm.org/D34823#795758" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D34823#795758</a>, @dberlin wrote:<br>
>><br>
>> > Without a reason to do this specifically, it seems a waste of time<br>
>> ><br>
>> > None of this data is used after initial building, and all of these are<br>
>> > destroyed after newgvn ends, so updating them doesn't seem to accomplish<br>
>> > anything?<br>
>><br>
>><br>
>> I was making the Instruction * in OrdereBasicBlock AssertingVH to make<br>
>> sure things are updated properly when instructions are deleted. If they are<br>
>> not, we are essentially holding a dangling pointer which could in worst case<br>
>> result in invalid dominance relationship being returned. Then i found NewGVN<br>
>> is breaking the rule, i.e. erasing an instruction without updating<br>
>> OrderedBasicBlock in OrderedInstruction.<br>
>><br>
>> I agree the way NewGVN is written now is ok even without updating the<br>
>> OrderedBasicBlock. Maybe we should not make Instruction * in OBB<br>
>> AssertingVH. Instead should give user the freedom to invalidate if<br>
>> necessary.<br>
>><br>
>><br>
>> <a href="https://reviews.llvm.org/D34823" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D34823</a><br>
>><br>
>><br>
>><br>
><br>
<br>
<br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Software Engineer - Compiler Toolchain<br>
Employee of Facebook Inc.<br>
</font></span></blockquote></div><br></div>