[PATCH] D87847: [AMDGPU] global-isel support for RT

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUInstructionSelector.cpp:3059-3075
+  if (IsA16) {
+    MIB.addReg(RayDir, 0, AMDGPU::sub0);
+    Register R1 = MRI->createVirtualRegister(&AMDGPU::VGPR_32RegClass);
+    Register R2 = MRI->createVirtualRegister(&AMDGPU::VGPR_32RegClass);
+    BuildMI(*MBB, &*MIB, DL, TII.get(AMDGPU::V_PACK_B32_F16), R1)
+      .addImm(0)
+      .addReg(RayDir, 0, AMDGPU::sub1)
----------------
rampitec wrote:
> arsenm wrote:
> > rampitec wrote:
> > > arsenm wrote:
> > > > Can you do this during custom lowering rather than adding bit operations here late? I'm also surprised a V_PACK_B32_F16 is involved here
> > > What kind of operations you'd like to see in the custom lowering? v_pack_b32_f16 should be fine, this is packed half type in this case.
> > But v_pack_b32_f16 isn't semantically the same as the bit packing, so I would be surprised to insert this for the argument handling.
> That's the best instruction for the job IMO. What we are doing is repacking vector of halfs.
But it does change the input values. I believe this is a canonicalizing operation, so may flush denorms and quiet snans


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

https://reviews.llvm.org/D87847