[PATCH] D93189: Introduce the `!nocapture` metadata and "nocapture_use" operand bundle

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.
Herald added a reviewer: bollu.

This isn't a comment on the proposed design so much as how the proposed design is described.  (I intend to respond to your llvm-dev thread... at some point.)

I think you need to revise the specification wording into something more operational.  As currently described, you define metadata in terms of existing analyzes which aren't themselves well defined.  (e.g. What *precisely* does it mean for something to be captured?)  What operations are allowed and/or disallowed by the existence of the metadata/bundle?  For the ones which are disallowed, is that full UB?  Producing poison?  Something else?

Entirely seriously, I'm not sure I could implement the current specification.  In particular, I don't believe I could take this specification and implement it in another compiler without *very* close examination of LLVM's current implementation details.  As one of the people who will end up maintaining this, having to back reference the implementation of a N-year old compiler to understand current semantics will rapidly become very problematic.

Here's an attempt to take what you've written for the !nocapture metadata and translate it into something a bit more operation.  Your welcome to take this as either a starting point, or as simply an illustration of what I meant by operational.

The existence of the !nocapture metadata on the instruction indicates that the location stored to is not captured.  That is, there exists a small set of frame local copies of points which can be used to access that location, and that if the optimizer can identify all of them, it can use that information to contribute to tracking all copies of the stored pointer.

If the storage location has been captured, execution of a store w/the !nocapture metadata is immediate UB.  Storing a previously captured pointer into an uncaptured location is well defined.  That is, the metadata says nothing of the capture status for the stored value.

On it's own, a !nocapture store says nothing about whether the contents of storage are later loaded, and the pointer stored later propagated.  However, !nocapture stores and the nocapture_use operand bundle can be combined to provide this stronger fact.

Defect List -- These are things I know are wrong with my attempt.

"frame local" needs defined, and might not be quite what we want.

"captured" is loosely defined, and needs revision.

After my own attempt, one further suggestion on structuring the langref changes.  I think we need to introduce a definitional section defining capture, and spelling out implications of that, then having the inline description of the metadata be purely structural and link back.  Doing it all inline in the store section seems really forced.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93189