[PATCH] D87827: [SCEVExpander] Support expanding nonintegral pointers with constant base.
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 18 02:50:42 PDT 2020
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
More information about the llvm-commits
mailing list