[PATCH] D143437: [llvm] Use pointer index type for more GEP offsets (pre-codegen)

[Krzysztof Drewniak via Phabricator via llvm-commits]

krzysz00 added a comment.

@arichardson  I'd like to also draw your attention to https://reviews.llvm.org/D143526, its global isel equivalent.

It'd be nice to fix up SelectionDAG to also generate `ADD` nodes using the index width and not the data width, but that's not what I'm trying first.



================
Comment at: llvm/lib/Transforms/Scalar/NaryReassociate.cpp:454
+  if (RHS->getType() != PtrIdxTy)
+    RHS = Builder.CreateSExtOrTrunc(RHS, PtrIdxTy);
   if (IndexedSize != ElementSize) {
----------------
foad wrote:
> Not necessarily related to your patch, but the "OrTrunc" makes me nervous (here and in a couple of other places in this patch). My understanding of the index type in datalayout is that it is a preferred type to use GEP indexes, but shouldn't actually have any semantic effect, so it would be wrong to truncate an index to the index type unless you can prove that it won't change the value.
That's not my understanding of that field. The index field is maximal size of an offset into a pointer in address space N. For example, CHERI's capability pointers are (to my understanding) `128:128:128:64` meaning that the pointer value is 128 bits long, but only 64 bits of offset can be applied to it.

That is, in C terms, the pointer type is `intptr_t` and the index field is `size_t`. This patch fixes up a bunch of code that normalized pointer offsets to `intptr_t` instead of `size_t`, since LLVM historically assumed they were the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143437