[PATCH] D41652: [InstCombine] Add an option to disable addrspacecast folding into GEP

[Marek Olšák via Phabricator via llvm-commits]

mareko added a comment.

In https://reviews.llvm.org/D41652#965806, @davide wrote:

> In https://reviews.llvm.org/D41652#965797, @mareko wrote:
>
> > In https://reviews.llvm.org/D41652#965765, @davide wrote:
> >
> > > I'm not sure having a `cl::opt` is the best option here.
> > >  If you really want to make such a change, can you at least thread this information through `TargetTransformInfo` rather than using a global option?
> >
> >
> > Yes.
> >
> > > Also, for my own curiosity, is this a temporary workaround until AMDGPU grows proper support for 32-bit GEPs or is this an inherent limitation?
> > >  If the former, maybe this hack should not go in (or at least we should consider what's the amount of work needed to finish implementing the support)
> >
> > It's possible that this will be a permanent solution, because we don't plan to have full 32-bit support (it would be too much work in the backend for little benefit).
>
>
> This is quite unfortunate. I'd like to point out I don't feel particularly comfortable to have this as a long term solution (but, maybe, OK for 6.0 & reverted in trunk).
>  The contract between the mid-level optimizer and the backend is that the latter should possibly accept everything produced by the former (or, FWIW, error out in some circumstances).
>  Maybe you can have something in your backend logic that recovers from the fact that AMDGPU doesn't know (and won't be taught) about 32-bit GEPs? 
>  Have you considered something during legalization? [Apologies if I'm off, but I'm not really familiar with the way AMDGPU works, at least not in depth].


GEPs will actually work, but the generated assembly for loads will be worse, thus I'd like to avoid them. A better solution for LLVM 7.0 might be possible.


https://reviews.llvm.org/D41652