[PATCH] D86154: AMDGPU: Add llvm.amdgcn.{read,readfirst,write}lane2 intrinsics with type overloads

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 27 06:57:34 PDT 2020


arsenm added a comment.

In D86154#2229292 <https://reviews.llvm.org/D86154#2229292>, @nhaehnle wrote:

> In D86154#2224272 <https://reviews.llvm.org/D86154#2224272>, @arsenm wrote:
>
>> In D86154#2224270 <https://reviews.llvm.org/D86154#2224270>, @nhaehnle wrote:
>>
>>> Note that part of my motivation here over D84639 <https://reviews.llvm.org/D84639> is to support more general types on the lane intrinsics, since they also express some semantic content which would be interesting to be able to express e.g. on descriptors. I wasn't able to bend the SelectionDAG type legalization to my will, so that's why I instead "legalize" the intrinsics in the AMDGPUCodeGenPrepare pass.
>>
>> Don't you just need to handle this in ReplaceNodeResults the same way?
>
> ReplaceNodeResults expects the result type to be changed in semi-magical ways during vector type legalization, which is non-obvious since the method can be called from different places. I think it *could* be made to work with a lot of patience, but it's really a bad interface -- and besides, by doing it in IR we reduce code duplication between SelectionDAG and GlobalISel, which is an added benefit IMO.

Well the globalisel handling should be much simpler. We have a lot of stuff that's randomly handled in the IR to work around the DAG which long term should be moved where it belongs in codegen


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86154



More information about the llvm-commits mailing list