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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec 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)
----------------
arsenm wrote:
> 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
It should behave the same as bhv itself, the mode is common right? So if flushing on the value will be flushed anyway. Everything else results in a longer code. It can use v_lshl_or_b32, but it will also need an extra v_and_b32 to clear high half.


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

https://reviews.llvm.org/D87847