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

[Davide Italiano via Phabricator via llvm-commits]

davide added a comment.

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].


https://reviews.llvm.org/D41652