[PATCH] D87827: [SCEVExpander] Support expanding nonintegral pointers with constant base.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D87827#2280309 <https://reviews.llvm.org/D87827#2280309>, @efriedma wrote:

> I have a proposal in https://bugs.llvm.org/show_bug.cgi?id=46786#c20 which would solve this, among other issues.  Not sure when I'll get around to actually implementing it.

This looks very interesting, but like a non-trivial amount of work. The expander problem is currently blocking the re-landing of D71539 <https://reviews.llvm.org/D71539> and it seems the fix in the expander is relatively small/straight-forward and it would be great to unblock that change :)



================
Comment at: llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp:134
+  // the reminder of the division will be stripped.
+  if (Op == Instruction::IntToPtr && DL.isNonIntegralPointerType(PtrTy)) {
+    auto *Idx = Builder.CreateUDiv(
----------------
efriedma wrote:
> fhahn wrote:
> > arsenm wrote:
> > > It seems broken that getCastOpcode would return IntToPtr in a situation where it's illegal
> > Yes, that seems a rather big hole in getCastOpcode unfortunately and should probably also be changed.
> > 
> > But I think the key question is how to best deal with the problem during SCEV expansion here, because AFAIK we do not have a way to indicate that we failed to expand an expression in the expander.
> > 
> > Is it enough to emit a `GEP` with a computed index? Or should we better try to get a type with an alloc size of 1 byte and use that? I guess there could be configurations which do not have such a type, not sure what to do then.
> In general, for non-null pointers, SCEVExpander uses a GEP over i8* (in the appropriate address space).  We should do that even if the implicit null is missing.  The offset isn't necessarily divisible by the element type: SCEV looks through bitcasts.
> 
> > I guess there could be configurations which do not have such a type
> 
> i8 is always one byte.  (This should be true even with an out-of-tree patchset that makes bytes bigger than 8 bits: it would just be a non-byte-sized type, similar to i1.)
> In general, for non-null pointers, SCEVExpander uses a GEP over i8* (in the appropriate address space). We should do that even if the implicit null is missing. The offset isn't necessarily divisible by the element type: SCEV looks through bitcasts.
> 
>> I guess there could be configurations which do not have such a type
> i8 is always one byte. (This should be true even with an out-of-tree patchset that makes bytes bigger than 8 bits: it would just be a non-byte-sized type, similar to i1.)

Oh, I thought there might be out-of-tree targets that have `i8` with different alloc sizes, but if that is nothing to worry about we can just emit a GEP with i8* null as base pointer. I updated the code and added an extra assertion to make things blow up if the assumption is broken.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87827