[PATCH] D105347: [AMDGPU][GlobalISel] Legalization and selection of G_ROTL and G_ROTR

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/VOP3Instructions.td:346
+  (rotr i32:$src0, i32:$src1),
+  (V_ALIGNBIT_B32_e64 VGPR_32:$src0, VGPR_32:$src0, VGPR_32:$src1)
+>;
----------------
matejam wrote:
> foad wrote:
> > arsenm wrote:
> > > Isn't there a lowering to turn this into fshr? Why do we need to directly select this?
> > I have previously discussed this with Mateja. There's a combine that turns funnel shifts into rotates, so I wasn't sure if that meant that the rotates were canonical and we had to handle them.
> > 
> > Thinking about it now, I guess we can say that rotates are illegal, and the combine should bail out in that case?
> If we disabled the "funnel shift to rotate" combiner with a check if the rotate is legal or not, we would also be disabling that combiner for AArch64, specifically for aarch64-prelegalizer-combiner pass.  
That combine //already// checks "is legal or before legalizer", so it should be OK. (It is a bit inefficient, because the pre-legalizer combiner will convert funnel shift -> rotate and then the legalizer will convert it back again, but I think that is OK.)


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

https://reviews.llvm.org/D105347