[PATCH] D62775: [SelectionDAG] Skip addrspacecast expansion when casting undef values

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.
Herald added a reviewer: Jim.

In D62775#1528629 <https://reviews.llvm.org/D62775#1528629>, @dylanmckay wrote:

> Thank you for looking over this @arsenm.
>
> In my particular case, the IR that was triggering the failure used an `undef` value. Now that you mention it, I can see that perhaps the `undef` was coincidental and the current fix leaves the general case of any `addrspacecast` unhandled.


Yes, I don't see the point of special casing avoiding a need to handle undef. A target may want to do something special for its undef fold I guess? If you aren't handling the entire operation, there's not much benefit to just undef working.

> After reading the LangRef docs on addrspacecast <https://llvm.org/docs/LangRef.html#addrspacecast-to-instruction>, this line stuck out:
> 
>> Note that if the address space conversion is legal then both result and operand refer to the same memory location.
> 
> On AVR, there is no transformation you can do to a pointer to transform it into a pointer in a different address space, that still points at the same cells in memory - there is no MMU, paging, virtual memory or memory mapping (outside of IO registers and occasionally, EEPROMS). If you want to load from program space, you must use `lpm`, if you want to load from data space, you must use `ld`.
> 
> Does this mean that `addrspacecast`s should be illegal on AVR and not emitted by the frontend? Feels strange to have a backend silently supporting only a subset of the full IR gamut - perhaps the GPU backends do this too?
> 
> At the moment, the AVR backend does not handle `addrspacecast`s at all, always hitting an assertion during ISel.
> 
> What do you think would be the way forward?

For AMDGPU, we custom lower addrspacecast and emit an error for the illegal ones: https://github.com/llvm-mirror/llvm/blob/master/lib/Target/AMDGPU/SIISelLowering.cpp#L4396


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62775