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

[Mateja Marjanovic via Phabricator via llvm-commits]

matejam added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:6088
+    if (!IsFShLegal) {
+      if (isPowerOf2_32(EltSizeInBits)) {
+        FShOpc = RevFsh;
----------------
foad wrote:
> What if it's not a power of two? In that case, should we fall through to the G_SHL/G_LSHR expansion below?
Yes, we should... we go into the "if (!IsFShLegal) " statement only if the instruction with the opcode FShOpc isn't legal and the other is. The problem is if the RevFsh instruction is legal and the FShOpc instruction isn't and the type isn't a power of 2, this code will try to build an instruction which ins't legal, instead of falling through to the G_SHL/G_LSHR part.
I'll fix that right away.


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

https://reviews.llvm.org/D105347