[PATCH] D134308: AtomicExpand: Use llvm.ptrmask instead of ptrtoint

[James Y Knight via Phabricator via llvm-commits]

jyknight added a comment.

In D134308#3809031 <https://reviews.llvm.org/D134308#3809031>, @arsenm wrote:

> In D134308#3808877 <https://reviews.llvm.org/D134308#3808877>, @arsenm wrote:
>
>> In D134308#3808841 <https://reviews.llvm.org/D134308#3808841>, @jyknight wrote:
>>
>>> is incorrect. Sorry to be a pain, but do you have a link that describes the issues here? I may well not be remembering properly, but I had thought previous discussions and changes were about pointer provenance issues that arise from the inttoptr direction.
>>
>> ptrtoint is the most basic capture <https://github.com/llvm/llvm-project/blob/4132bce9e56b00cdce8928e4ea67b136c93f46a2/llvm/lib/Analysis/CaptureTracking.cpp#L432>

OK.

> Also, just see the langref description for llvm.ptrmask

I looked at that first thing. It does not -- at least, not obviously -- discuss this side of the issue. It describes its semantics as being equivalent to `getelementptr ptr, (ptrtoint(ptr) & mask) - ptrtoint(ptr)`. It also says it preserves more information about the resulting pointer than ptrtoint+inttoptr. Both these statements //imply// that the issue it's solving is the loss of information that occurs via creation of a new pointer with `inttoptr` -- and that ptrmask exists primarily because it's a much simpler canonical form to deal with than the equivalent GEP -- which requires extraneous addition/subtraction operations.

Anyways, I have no issue with this change itself (only the 1 minor nit) -- just with the description.


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

https://reviews.llvm.org/D134308