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

[Harald van Dijk via Phabricator via llvm-commits]

hvdijk 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
----------------
Nit: cast is an irregular verb, "casted" should be "cast". (The same mistake is already present in other places.)


================
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.
----------------
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`.


================
Comment at: llvm/docs/LangRef.rst:2703
+stored in the memory by loading it as a non-pointer type loses its associated
+address range.
+
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104013