[PATCH] D61607: Introduce an option to stripPointerCasts to force the same bit pattern

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 21 07:55:45 PDT 2019


hfinkel added a comment.

In D61607#1509496 <https://reviews.llvm.org/D61607#1509496>, @jdoerfert wrote:

> In D61607#1507697 <https://reviews.llvm.org/D61607#1507697>, @aykevl wrote:
>
> > Yes I know it can change the bit pattern. And if it is only about the bit pattern, the name is fine. However, it seems to me that the only difference between `stripPointerCastsSameBitPattern` and `stripPointerCasts` is the AS, which may or may not be relevant to the bit pattern. This is confusing: it isn't clear to me from the name `stripPointerCastsSameBitPattern` whether it may strip any `addrspacecast` that has the same bit pattern.
>
>
> I think I do not understand the problem. For me, it is all about the bit-pattern. But in general, I think users should not care what casts are stripped and which are not. It should be clear from the name and description what they can expect. Maybe someone adds logic here to determine if as-casts preserve the bit pattern. That will help users that need the same bit pattern, e.g., the `isNonNull` family of calls, and it will break with the strict no-as rule. If we add another "as-cast-like" instruction (like D59065 <https://reviews.llvm.org/D59065>) that might change the bit-pattern but not the "logical object" users of any `stripPointerCastsXXX` call should get the right value.


I agree with this. I recall past conversations about adding something to DataLayout to say that some address-space pairs share the same pointer representation, or some aspects of the representation, I don't know if we ever had a strong-enough use case to write the code, but should such a worthwhile case emerge, I can certainly see us teaching the optimizer that some as-casts don't change (certain relevant aspects of) the underlying representation.

As a bit of bikeshedding, I like the term 'representation' better than 'pattern'. I wonder if the name `stripPointerCastsSameIntRepresentation` would be clearer? (The idea being that pointers are opaque types at the IR level, but get integer representations via ptrtoint).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61607





More information about the llvm-commits mailing list