[PATCH] D109228: [AMDGPU][GlobalISel] Legalize G_MUL for non-standard types

[Jay Foad via Phabricator via llvm-commits]

foad accepted this revision.
foad added a comment.
This revision is now accepted and ready to land.

LGTM, thanks! Please update the commit message which still mentions clampScalar.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:552
+      .minScalar(0, S16)
+      .widenScalarToNextMultipleOf(0, 32)
+      .maxScalar(0, S32)
----------------
mbrkusanin wrote:
> foad wrote:
> > For this case (have 16 insts but no 16 bit packed insts) I think the order should be something like:
> > ```
> >       .legalFor({S32, S16})
> >       .minScalar(0, S16)
> >       .scalarize(0)
> >       .widenScalarToNextMultipleOf(0, 32)
> >       .maxScalar(0, S32)
> > ```
> > Otherwise v2s16 would be widened to v2s32 and then scalarized to s32. Instead we want to scalarize it to s16.
> llvm/test/CodeGen/AMDGPU/GlobalISel/mul.v2i16.ll has those tests and current version of the patch does not seem to affect it.
> It is still scalarized to s16.
> 
> Either way //scalarize()// will be called first. //widenScalarToNextMultipleOf()// checks if type is scalar.
> Either way scalarize() will be called first. widenScalarToNextMultipleOf() checks if type is scalar.

Oh yes, I missed that. This is OK then.


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

https://reviews.llvm.org/D109228