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

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 29 13:35:27 PDT 2017


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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170629/2ed45d10/attachment.html>


More information about the llvm-commits mailing list