[PATCH] D83214: [AMDGPU][GlobalISel] Select llvm.amdgcn.ballot

[Matt Arsenault via Phabricator via llvm-commits]

arsenm requested changes to this revision.
arsenm added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:1054-1059
+  MachineInstr *Value = MRI->getVRegDef(I.getOperand(2).getReg());
+  if (Value->getOpcode() == AMDGPU::COPY)
+    Value = MRI->getVRegDef(Value->getOperand(1).getReg());
+
+  if (Value->getOpcode() == AMDGPU::G_CONSTANT) {
+    const APInt &Val = Value->getOperand(1).getCImm()->getValue();
----------------
You want getConstantVRegVal instead of looking through a copy and checking for G_CONSTANT


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:1062
+    if (Val.isNullValue()) {
+      unsigned Opcode = Is64 ? AMDGPU::V_MOV_B32_e64 : AMDGPU::V_MOV_B32_e32;
+      BuildMI(*BB, &I, DL, TII.get(Opcode), DstReg).addImm(0);
----------------
This doesn't make any sense; there's no reason to ever use the VOP3 encoded form of v_mov_b32. It's nota  64-bit move.

This also returns a scalar value


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:1065
+    } else if (Val.isAllOnesValue()) {
+      unsigned SrcReg = Is64 ? AMDGPU::EXEC : AMDGPU::EXEC_LO;
+      BuildMI(*BB, &I, DL, TII.get(AMDGPU::COPY), DstReg).addReg(SrcReg);
----------------
Register


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:4165
+      unsigned SrcSize = MRI.getType(MI.getOperand(2).getReg()).getSizeInBits();
+      OpdsMapping[0] = AMDGPU::getValueMapping(AMDGPU::VGPRRegBankID, DstSize);
+      OpdsMapping[2] = AMDGPU::getValueMapping(AMDGPU::VCCRegBankID, SrcSize);
----------------
This returns an SGPR value


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83214