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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:1053-1054
+
+  Optional<ValueAndVReg> Arg =
+      getConstantVRegValWithLookThrough(I.getOperand(2).getReg(), *MRI, true);
+
----------------
I think you want just regular getConstantVRegVal. I don't think you're getting much from the look through


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:1059
+    if (Value == 0) {
+      BuildMI(*BB, &I, DL, TII.get(AMDGPU::S_MOV_B32), DstReg).addImm(0);
+    } else if (Value == -1) { // all ones
----------------
This would need to be an S_MOV_B64 for wave 64?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:1064
+    } else
+      return false;
+  } else {
----------------
This should be unreachable code (however, the verifier doesn't check intrinsic operand types so I guess you have to leave this)


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.ballot.i64.ll:11-12
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_mov_b32 s0, 0
+; CHECK-NEXT:    s_mov_b32 s1, 0
+; CHECK-NEXT:    ; return to shader part epilog
----------------
This can be one s_mov_b64


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.ballot.i64.ll:23-24
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_mov_b32 s0, exec_lo
+; CHECK-NEXT:    s_mov_b32 s1, exec_hi
+; CHECK-NEXT:    ; return to shader part epilog
----------------
One s_mov_b64


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

https://reviews.llvm.org/D83214