[PATCH] D34823: [PredicateInfo] Invalidate an OrderedBasicBlock when instruction in it is deleted.

[Xin Tong via llvm-commits]

Because analysis might be used by later passes ?
-Xin

On Fri, Jun 30, 2017 at 5:35 AM, Daniel Berlin <dberlin at dberlin.org> wrote:
> I don't think we should make utilities use AssertingVH like this, at least
> by default.  Utilities are often built and thrown away.  If it was an
> analysis, I'd completely agree we should use AssertingVH
>
>
> On Thu, Jun 29, 2017, 1:33 PM Xin Tong via Phabricator
> <reviews at reviews.llvm.org> wrote:
>>
>> trentxintong added a comment.
>>
>> In https://reviews.llvm.org/D34823#795758, @dberlin wrote:
>>
>> > Without a reason to do this specifically, it seems a waste of time
>> >
>> > None of this data is used after initial building, and all of these are
>> > destroyed after newgvn ends, so updating them doesn't seem to accomplish
>> > anything?
>>
>>
>> I was making the Instruction * in OrdereBasicBlock AssertingVH to make
>> sure things are updated properly when instructions are deleted. If they are
>> not, we are essentially holding a dangling pointer which could in worst case
>> result in invalid dominance relationship being returned. Then i found NewGVN
>> is breaking the rule, i.e. erasing an instruction without updating
>> OrderedBasicBlock in OrderedInstruction.
>>
>> I agree the way NewGVN is written now is ok even without updating the
>> OrderedBasicBlock. Maybe we should not make Instruction * in OBB
>> AssertingVH. Instead should give user the freedom to invalidate if
>> necessary.
>>
>>
>> https://reviews.llvm.org/D34823
>>
>>
>>
>



-- 
Software Engineer - Compiler Toolchain
Employee of Facebook Inc.