[PATCH] D73446: AMDGPU/GlobalISel: Legalize a16 images

Nicolai Hähnle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 29 01:08:55 PST 2020


nhaehnle added a comment.

In D73446#1842457 <https://reviews.llvm.org/D73446#1842457>, @arsenm wrote:

> In D73446#1842443 <https://reviews.llvm.org/D73446#1842443>, @arsenm wrote:
>
> > In D73446#1842422 <https://reviews.llvm.org/D73446#1842422>, @nhaehnle wrote:
> >
> > > Why is this conceptually a legalization rather than part of the instruction selection?
> >
> >
> > The register layout packing/unpacking code should be exposed to the legalizer and combines. Doing it later means it could possibly be in a waterfall loop, and won't benefit from the combines on the pack/unpack code.
>
>
> More abstractly, selection should operate on the types expected for the final instruction. It's the legalizer's job to get registers that are the right type/size


The part that makes me uncomfortable here and I couldn't quite put a finger on initially is that it implies a change in the semantics of the intrinsic. What this and related changes are doing is implicitly changing the design such that the intrinsics mean something different before vs. after legalization. That is not what legalization is usually supposed to do.

I see your point about combiner passes -- so could we perhaps just select the image instructions early instead? It's not like we have very interesting things happening for image instructions in the ISel patterns anyway, and one of the benefits of the GlobalISel infrastructure is that it's supposed to be flexible enough for stuff like that...


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

https://reviews.llvm.org/D73446





More information about the llvm-commits mailing list