[PATCH] D108741: [IR][GlobalDCE] Add ability to mark vtable methods as eligible for VFE and avoid eliminating non-eligible vfunc in VFE in GlobalDCE

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

I did skim the thread, and it seemed to me that Peter's objection was largely to the reuse of `!type`, which he feels wasn't meant to carry these semantics.  Peter leapt to the idea of a new constant instead of a more general metadata, but I don't understand what that leap is meant to achieve, which is why I'm engaging with the thread.

My objection to the new constant is that it doesn't seem to have any independent meaning, it's just a way of identifying particular fields in a global initializer for the purposes of a specific optimization.  If you end up with a use of this constant as an instruction operand, it's meaningless and has to be stripped.  So I'm not really sure how it reinforces correctness.

As far as I understand it, your optimization is basically: certain global objects have their addresses escape in ways you can't directly reason about, but you know that certain fields of those objects can only be accessed by corresponding blessed operations, and you can sometimes prove that some blessed operations don't exist in the program, which means you can prove that those fields are irrelevant, which enables various optimizations.  Within that setup, you really only have one correctness problem, which is that it's vitally important that *accesses* be reliably marked as blessed, which the constant does nothing to address: failing to mark a global object reliably just introduces a failure to optimize, which is not a correctness problem.  You also have a *expressiveness* problem, which is that you're treating the entire object uniformly instead of tracking blessed operations field-by-field, which will create correctness problems if you try to apply the optimization to objects which don't restrict access to all the fields.  But I don't see how the new constant actually helps there at all; really you need finer-grained "blessings", both on access sites and in the object definition, and the constant doesn't carry those annotations, so...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108741/new/

https://reviews.llvm.org/D108741