[PATCH] D104013: [LangRef] State that the based-on relation is for pointer typed values only

Nicolai Hähnle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 12 00:28:38 PDT 2021


nhaehnle added inline comments.


================
Comment at: llvm/docs/LangRef.rst:2665
+   in *any* address-space is associated with no address range.
+-  A pointer casted from an integer constant other than zero or a pointer value
+   returned from a function not defined within LLVM may be associated with
----------------
hvdijk wrote:
> Nit: cast is an irregular verb, "casted" should be "cast". (The same mistake is already present in other places.)
Some targets have address spaces in which 0 is a valid pointer, so this should probably be adjusted while we're at it? It depends on the address space.


================
Comment at: llvm/docs/LangRef.rst:2689
+   to assume that ``q`` is based on ``p`` because the value of ``i`` could have
+   been contributed by another pointer via conditional branches as well.
 -  The "*based* on" relationship is transitive.
----------------
hvdijk wrote:
> This looks wrong, `q` is based on "all pointer values that contribute (directly or indirectly) to the computation of the pointer's value", and `p`'s value clearly contributes to that of `q`, so `q` is based on `p`. What I think you're trying to say here is that it is incorrect to assume that `q` is //only// based on `p`.
I agree. I wonder if this bullet point couldn't be folded into the bullet "A pointer cast from an integer constant ..." above.

Is there a good reason for distinguishing between an inttoptr from constant vs. inttoptr from anything else? I'd naively assume they behave the same.


================
Comment at: llvm/docs/LangRef.rst:2703
+stored in the memory by loading it as a non-pointer type loses its associated
+address range.
+
----------------
hvdijk wrote:
> This implies that storing a pointer value in memory and loading it as a pointer type does preserve its address range. Whether it does is unclear; this idea is assumed by some existing optimisations and incompatible with other existing optimisations. In my opinion, it is not a good idea to update the documentation to clarify this either way until it is decided how LLVM should behave.
> 
> If you have raised this not to update the documentation directly, but only to as a starting point for discussion, could you also include what happens when an integer is stored to memory and is then loaded as a pointer value?
> This implies that storing a pointer value in memory and loading it as a pointer type does preserve its address range. Whether it does is unclear; this idea is assumed by some existing optimisations and incompatible with other existing optimisations

If store of pointer followed by load of pointer *doesn't* preserve the address range, then mem2reg is likely incorrect as-is, so... :)

Which existing optimizations are incompatible with this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104013



More information about the llvm-commits mailing list