[PATCH] D74928: AMDGPU/GlobalISel: Fix handling of G_ANYEXT with s1 source

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

Looks OK but I'm a bit out of my depth so I'd prefer someone else to take a look too.



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:2159
     LLT SrcTy = MRI.getType(SrcReg);
-    bool Signed = Opc == AMDGPU::G_SEXT;
+    const bool Signed = Opc == AMDGPU::G_SEXT;
+
----------------
This would be clearer folded into its only use on line 2180.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:2205
+    // It is not legal to have a legalization artifact with a VCC source. Rather
+    // than introducing a copy, insert the selcet we would have to select the
+    // copy to.
----------------
Typo "selcet".


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-anyext.mir:25
   bb.0:
     liveins: $vgpr0_vgpr1
     ; CHECK-LABEL: name: anyext_s32_to_s64_v
----------------
Only vgpr0 is live-in, but I guess this is harmless?


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/regbankselect-anyext.mir:28
     ; CHECK: [[COPY:%[0-9]+]]:vgpr(s32) = COPY $vgpr0
-    ; CHECK: [[ANYEXT:%[0-9]+]]:vgpr(s64) = G_ANYEXT [[COPY]](s32)
+    ; CHECK: [[COPY1:%[0-9]+]]:vgpr(s32) = COPY [[COPY]](s32)
+    ; CHECK: [[DEF:%[0-9]+]]:vgpr(s32) = G_IMPLICIT_DEF
----------------
Why do we get the extra copy? Is this just more cruft that should be cleaned up by a later pass?


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

https://reviews.llvm.org/D74928