[PATCH] D104268: [ptr_provenance] Introduce optional ptr_provenance operand to load/store

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

In D104268#3266830 <https://reviews.llvm.org/D104268#3266830>, @jeroen.dobbelaere wrote:

> In D104268#3266800 <https://reviews.llvm.org/D104268#3266800>, @nikic wrote:
>
>> It's been a while since I looked at this, so please excuse the stupid question...
>>
>> Why do we model provenance as an additional value on the load/store? Naively, I would think that it would be preferable to have an intrinsic like `p' = llvm.with.provenance(p, provenance)`, because that would work with any use of the pointer -- the ptr_provenance operand is limited to loads and stores, but not with memory-accessing calls or captured pointers, and I don't think it can really be extended in that direction.
>
> Hi Nikita,
>
> my talk on the 2021 LLVM Dev Conference explains this. See https://www.youtube.com/watch?v=08XwXB3GHck 2021 LLVM Dev Mtg “ptr_provenance and @llvm.noalias: The Tale of Full Restrict”
>
> In short:
>
> - there is the `llvm.with.provenance(p, provenance)` equivalent intrinsic: it is called `llvm.experimental.ptr.provenance` (see D107355 <https://reviews.llvm.org/D107355>)
> - load/store instructions are handled differently, mostly to make the process more efficient and to make the new support easier to integrate with existing optimization passes. Those that just replace the pointer computation won't influence the provenance (once that is set).

Thanks! So the ptr_provenance is just an optimized representation of the intrinsic, that makes sense.

Have you considered inverting this patch stack and landing the intrinsic first, as that part seems more straightforward than the IR change? I have a suspicion that actually supporting independent address + provenance will require some significant work for various users of getUnderlyingObject(), which may not account for the possibility of address and provenance being different. Updating all users seems like something we can do using just the intrinsic.


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

https://reviews.llvm.org/D104268