AMDGPU/GlobalISel: Insert readfirstlane on SGPR returns

[arsenm]

Summary

In case the source value ends up in a VGPR, insert a readfirstlane to
avoid producing an illegal copy later. If it turns out to be
unnecessary, it can be folded out.

[arsenm]

Remove workarounds in test

[nhaehnle]

Please remove the changes in the bswap test. It is critically important that we do not insert unnecessary v_readfirstlane insructions. Mesa performance leans on this quite heavily.

[nhaehnle]

Come to think of it, is there an explicit test that we *don't* insert v_readfirstlane when none is needed at all?

[arsenm]

In D74661#1878110, @nhaehnle wrote:

Please remove the changes in the bswap test. It is critically important that we do not insert unnecessary v_readfirstlane insructions. Mesa performance leans on this quite heavily.

This is just noise for the purpose of the bswap test. We already fold out unneeded. readfirstlanes in 3 different places which already have tests. I’m also planning on adding another in the regbank aware combiner

[arsenm]

ping

[foad]

Looks OK to me if Nicolai is happy with it now.

[arsenm]

ping

[arsenm]

ping

[nhaehnle]

LGTM

[arsenm]

67cfbec7461312ce6c1a8f38ad4f45b7886d51ef

llvm/lib/Target/AMDGPU/AMDGPUCallLowering.cpp

void assignValueToReg(Register ValVReg, Register PhysReg,
     Register ExtReg;
     if (VA.getLocVT().getSizeInBits() < 32) {
       // 16-bit types are reported as legal for 32-bit registers. We need to
       // extend and do a 32-bit copy to avoid the verifier complaining about it.
       ExtReg = MIRBuilder.buildAnyExt(LLT::scalar(32), ValVReg).getReg(0);
     } else
       ExtReg = extendRegister(ValVReg, VA);
 
+    // If this is a scalar return, insert a readfirstlane just in case the value
+    // ends up in a VGPR.
+    // FIXME: Assert this is a shader return.
+    const SIRegisterInfo *TRI
+      = static_cast<const SIRegisterInfo *>(MRI.getTargetRegisterInfo());
+    if (TRI->isSGPRReg(MRI, PhysReg)) {
+      auto ToSGPR = MIRBuilder.buildIntrinsic(Intrinsic::amdgcn_readfirstlane,
+                                              {MRI.getType(ExtReg)}, false)
+        .addReg(ExtReg);
+      ExtReg = ToSGPR.getReg(0);
+    }
+
     MIRBuilder.buildCopy(PhysReg, ExtReg);
     MIB.addUse(PhysReg, RegState::Implicit);
   }
 
   bool assignArg(unsigned ValNo, MVT ValVT, MVT LocVT,
                  CCValAssign::LocInfo LocInfo,
                  const CallLowering::ArgInfo &Info,
                  ISD::ArgFlagsTy Flags,

llvm/test/CodeGen/AMDGPU/GlobalISel/bswap.ll

 ; GFX9-LABEL: s_bswap_i32:
 ; GFX9:       ; %bb.0:
 ; GFX9-NEXT:    v_mov_b32_e32 v0, s0
 ; GFX9-NEXT:    s_mov_b32 s0, 0x10203
 ; GFX9-NEXT:    v_perm_b32 v0, 0, v0, s0
 ; GFX9-NEXT:    v_readfirstlane_b32 s0, v0
 ; GFX9-NEXT:    ; return to shader part epilog
   %bswap = call i32 @llvm.bswap.i32(i32 %src)
-  %to.sgpr = call i32 @llvm.amdgcn.readfirstlane(i32 %bswap)
-  ret i32 %to.sgpr
+  ret i32 %bswap
 }
 
 define i32 @v_bswap_i32(i32 %src) {
 ; GFX7-LABEL: v_bswap_i32:
 ; GFX7:       ; %bb.0:
 ; GFX7-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX7-NEXT:    v_alignbit_b32 v1, v0, v0, 8
 ; GFX7-NEXT:    v_alignbit_b32 v0, v0, v0, 24

 ; GFX9-NEXT:    s_mov_b32 s0, 0x10203
 ; GFX9-NEXT:    v_mov_b32_e32 v1, s1
 ; GFX9-NEXT:    v_perm_b32 v1, 0, v1, s0
 ; GFX9-NEXT:    v_perm_b32 v0, 0, v0, s0
 ; GFX9-NEXT:    v_readfirstlane_b32 s0, v0
 ; GFX9-NEXT:    v_readfirstlane_b32 s1, v1
 ; GFX9-NEXT:    ; return to shader part epilog
   %bswap = call <2 x i32> @llvm.bswap.v2i32(<2 x i32> %src)
-  %bswap.0 = extractelement <2 x i32> %bswap, i32 0
-  %bswap.1 = extractelement <2 x i32> %bswap, i32 1
-  %to.sgpr0 = call i32 @llvm.amdgcn.readfirstlane(i32 %bswap.0)
-  %to.sgpr1 = call i32 @llvm.amdgcn.readfirstlane(i32 %bswap.1)
-  %ins.0 = insertelement <2 x i32> undef, i32 %to.sgpr0, i32 0
-  %ins.1 = insertelement <2 x i32> %ins.0, i32 %to.sgpr1, i32 1
-  ret <2 x i32> %ins.1
+  ret <2 x i32> %bswap
 }
 
 define <2 x i32> @v_bswap_v2i32(<2 x i32> %src) {
 ; GFX7-LABEL: v_bswap_v2i32:
 ; GFX7:       ; %bb.0:
 ; GFX7-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX7-NEXT:    v_alignbit_b32 v2, v0, v0, 8
 ; GFX7-NEXT:    v_alignbit_b32 v0, v0, v0, 24

 ; GFX9-NEXT:    s_mov_b32 s4, 0x10203
 ; GFX9-NEXT:    v_perm_b32 v0, 0, v0, s4
 ; GFX9-NEXT:    v_perm_b32 v1, 0, v1, s4
 ; GFX9-NEXT:    s_setpc_b64 s[30:31]
   %bswap = call <2 x i32> @llvm.bswap.v2i32(<2 x i32> %src)
   ret <2 x i32> %bswap
 }
 
-define amdgpu_ps <2 x i32> @s_bswap_i64(i64 inreg %src) {
+define amdgpu_ps i64 @s_bswap_i64(i64 inreg %src) {
 ; GFX7-LABEL: s_bswap_i64:
 ; GFX7:       ; %bb.0:
 ; GFX7-NEXT:    v_alignbit_b32 v0, s1, s1, 8
 ; GFX7-NEXT:    v_alignbit_b32 v1, s1, s1, 24
 ; GFX7-NEXT:    s_mov_b32 s1, 0xff00ff
 ; GFX7-NEXT:    v_bfi_b32 v0, s1, v1, v0
 ; GFX7-NEXT:    v_alignbit_b32 v1, s0, s0, 8
 ; GFX7-NEXT:    v_alignbit_b32 v2, s0, s0, 24

 ; GFX9-NEXT:    s_mov_b32 s1, 0x10203
 ; GFX9-NEXT:    v_mov_b32_e32 v1, s0
 ; GFX9-NEXT:    v_perm_b32 v0, 0, v0, s1
 ; GFX9-NEXT:    v_perm_b32 v1, 0, v1, s1
 ; GFX9-NEXT:    v_readfirstlane_b32 s0, v0
 ; GFX9-NEXT:    v_readfirstlane_b32 s1, v1
 ; GFX9-NEXT:    ; return to shader part epilog
   %bswap = call i64 @llvm.bswap.i64(i64 %src)
-  %cast = bitcast i64 %bswap to <2 x i32>
-  %elt0 = extractelement <2 x i32> %cast, i32 0
-  %elt1 = extractelement <2 x i32> %cast, i32 1
-  %to.sgpr0 = call i32 @llvm.amdgcn.readfirstlane(i32 %elt0)
-  %to.sgpr1 = call i32 @llvm.amdgcn.readfirstlane(i32 %elt1)
-  %ins.0 = insertelement <2 x i32> undef, i32 %to.sgpr0, i32 0
-  %ins.1 = insertelement <2 x i32> %ins.0, i32 %to.sgpr1, i32 1
-  ret <2 x i32> %ins.1
+  ret i64 %bswap
 }
 
 define i64 @v_bswap_i64(i64 %src) {
 ; GFX7-LABEL: v_bswap_i64:
 ; GFX7:       ; %bb.0:
 ; GFX7-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX7-NEXT:    v_alignbit_b32 v2, v1, v1, 8
 ; GFX7-NEXT:    v_alignbit_b32 v1, v1, v1, 24

 ; GFX9-NEXT:    v_perm_b32 v2, 0, v1, s4
 ; GFX9-NEXT:    v_perm_b32 v1, 0, v0, s4
 ; GFX9-NEXT:    v_mov_b32_e32 v0, v2
 ; GFX9-NEXT:    s_setpc_b64 s[30:31]
   %bswap = call i64 @llvm.bswap.i64(i64 %src)
   ret i64 %bswap
 }
 
-define amdgpu_ps <4 x i32> @s_bswap_v2i64(<2 x i64> inreg %src) {
+define amdgpu_ps <2 x i64> @s_bswap_v2i64(<2 x i64> inreg %src) {
 ; GFX7-LABEL: s_bswap_v2i64:
 ; GFX7:       ; %bb.0:
 ; GFX7-NEXT:    v_alignbit_b32 v0, s1, s1, 8
 ; GFX7-NEXT:    v_alignbit_b32 v1, s1, s1, 24
 ; GFX7-NEXT:    s_mov_b32 s1, 0xff00ff
 ; GFX7-NEXT:    v_bfi_b32 v0, s1, v1, v0
 ; GFX7-NEXT:    v_alignbit_b32 v1, s0, s0, 8
 ; GFX7-NEXT:    v_alignbit_b32 v2, s0, s0, 24

 ; GFX9-NEXT:    v_perm_b32 v3, 0, v3, s1
 ; GFX9-NEXT:    v_perm_b32 v1, 0, v1, s1
 ; GFX9-NEXT:    v_readfirstlane_b32 s0, v0
 ; GFX9-NEXT:    v_readfirstlane_b32 s1, v1
 ; GFX9-NEXT:    v_readfirstlane_b32 s2, v2
 ; GFX9-NEXT:    v_readfirstlane_b32 s3, v3
 ; GFX9-NEXT:    ; return to shader part epilog
   %bswap = call <2 x i64> @llvm.bswap.v2i64(<2 x i64> %src)
-  %cast = bitcast <2 x i64> %bswap to <4 x i32>
-  %bswap.0 = extractelement <4 x i32> %cast, i32 0
-  %bswap.1 = extractelement <4 x i32> %cast, i32 1
-  %bswap.2 = extractelement <4 x i32> %cast, i32 2
-  %bswap.3 = extractelement <4 x i32> %cast, i32 3
-  %to.sgpr0 = call i32 @llvm.amdgcn.readfirstlane(i32 %bswap.0)
-  %to.sgpr1 = call i32 @llvm.amdgcn.readfirstlane(i32 %bswap.1)
-  %to.sgpr2 = call i32 @llvm.amdgcn.readfirstlane(i32 %bswap.2)
-  %to.sgpr3 = call i32 @llvm.amdgcn.readfirstlane(i32 %bswap.3)
-  %ins.0 = insertelement <4 x i32> undef, i32 %to.sgpr0, i32 0
-  %ins.1 = insertelement <4 x i32> %ins.0, i32 %to.sgpr1, i32 1
-  %ins.2 = insertelement <4 x i32> %ins.1, i32 %to.sgpr2, i32 2
-  %ins.3 = insertelement <4 x i32> %ins.2, i32 %to.sgpr3, i32 3
-  ret <4 x i32> %ins.3
+  ret <2 x i64> %bswap
 }
 
 define <2 x i64> @v_bswap_v2i64(<2 x i64> %src) {
 ; GFX7-LABEL: v_bswap_v2i64:
 ; GFX7:       ; %bb.0:
 ; GFX7-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX7-NEXT:    v_alignbit_b32 v4, v1, v1, 8
 ; GFX7-NEXT:    v_alignbit_b32 v1, v1, v1, 24

 
 define amdgpu_ps i16 @s_bswap_i16(i16 inreg %src) {
 ; GFX7-LABEL: s_bswap_i16:
 ; GFX7:       ; %bb.0:
 ; GFX7-NEXT:    s_lshl_b32 s1, s0, 8
 ; GFX7-NEXT:    s_and_b32 s0, s0, 0xffff
 ; GFX7-NEXT:    s_lshr_b32 s0, s0, 8
 ; GFX7-NEXT:    s_or_b32 s0, s0, s1
-; GFX7-NEXT:    s_bfe_u32 s0, s0, 0x100000
 ; GFX7-NEXT:    ; return to shader part epilog
 ;
 ; GFX8-LABEL: s_bswap_i16:
 ; GFX8:       ; %bb.0:
 ; GFX8-NEXT:    v_mov_b32_e32 v0, s0
 ; GFX8-NEXT:    s_mov_b32 s0, 0xc0c0001
 ; GFX8-NEXT:    v_perm_b32 v0, 0, v0, s0
 ; GFX8-NEXT:    v_readfirstlane_b32 s0, v0
 ; GFX8-NEXT:    ; return to shader part epilog
 ;
 ; GFX9-LABEL: s_bswap_i16:
 ; GFX9:       ; %bb.0:
 ; GFX9-NEXT:    v_mov_b32_e32 v0, s0
 ; GFX9-NEXT:    s_mov_b32 s0, 0xc0c0001
 ; GFX9-NEXT:    v_perm_b32 v0, 0, v0, s0
 ; GFX9-NEXT:    v_readfirstlane_b32 s0, v0
 ; GFX9-NEXT:    ; return to shader part epilog
   %bswap = call i16 @llvm.bswap.i16(i16 %src)
-  %zext = zext i16 %bswap to i32
-  %to.sgpr = call i32 @llvm.amdgcn.readfirstlane(i32 %zext)
-  %trunc = trunc i32 %to.sgpr to i16
-  ret i16 %trunc
+  ret i16 %bswap
 }
 
 define i16 @v_bswap_i16(i16 %src) {
 ; GFX7-LABEL: v_bswap_i16:
 ; GFX7:       ; %bb.0:
 ; GFX7-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX7-NEXT:    v_and_b32_e32 v1, 0xffff, v0
 ; GFX7-NEXT:    v_lshlrev_b32_e32 v0, 8, v0

 ; GFX9-LABEL: s_bswap_v2i16:
 ; GFX9:       ; %bb.0:
 ; GFX9-NEXT:    v_mov_b32_e32 v0, s0
 ; GFX9-NEXT:    s_mov_b32 s0, 0x2030001
 ; GFX9-NEXT:    v_perm_b32 v0, 0, v0, s0
 ; GFX9-NEXT:    v_readfirstlane_b32 s0, v0
 ; GFX9-NEXT:    ; return to shader part epilog
   %bswap = call <2 x i16> @llvm.bswap.v2i16(<2 x i16> %src)
-  %cast0 = bitcast <2 x i16> %bswap to i32
-  %to.sgpr = call i32 @llvm.amdgcn.readfirstlane(i32 %cast0)
-  ret i32 %to.sgpr
+  %cast = bitcast <2 x i16> %bswap to i32
+  ret i32 %cast
 }
 
 define i32 @v_bswap_i16_zext_to_i32(i16 %src) {
 ; GFX7-LABEL: v_bswap_i16_zext_to_i32:
 ; GFX7:       ; %bb.0:
 ; GFX7-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX7-NEXT:    v_and_b32_e32 v1, 0xffff, v0
 ; GFX7-NEXT:    v_lshlrev_b32_e32 v0, 8, v0

 ; GFX9-NEXT:    v_and_b32_e32 v1, 0xffff, v1
 ; GFX9-NEXT:    s_setpc_b64 s[30:31]
   %trunc = trunc i64 %src to i48
   %bswap = call i48 @llvm.bswap.i48(i48 %trunc)
   %zext = zext i48 %bswap to i64
   ret i64 %zext
 }
 
-declare i32 @llvm.amdgcn.readfirstlane(i32) #0
 declare i16 @llvm.bswap.i16(i16) #1
 declare <2 x i16> @llvm.bswap.v2i16(<2 x i16>) #1
 declare <3 x i16> @llvm.bswap.v3i16(<3 x i16>) #1
 declare i32 @llvm.bswap.i32(i32) #1
 declare <2 x i32> @llvm.bswap.v2i32(<2 x i32>) #1
 declare i64 @llvm.bswap.i64(i64) #1
 declare <2 x i64> @llvm.bswap.v2i64(<2 x i64>) #1
 declare i48 @llvm.bswap.i48(i48) #1
 
 attributes #0 = { convergent nounwind readnone }
 attributes #1 = { nounwind readnone speculatable willreturn }

llvm/test/CodeGen/AMDGPU/GlobalISel/irtranslator-amdgpu_ps.ll

define amdgpu_ps void @disabled_input_struct(float inreg %arg0, { float, float } %psinput0, float %psinput1) #1 {
   ; CHECK:   [[COPY2:%[0-9]+]]:_(s32) = COPY [[DEF]](s32)
   ; CHECK:   G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.amdgcn.exp), 0, 15, [[COPY]](s32), [[COPY]](s32), [[COPY]](s32), [[COPY1]](s32), 0, 0
   ; CHECK:   S_ENDPGM 0
 main_body:
   call void @llvm.amdgcn.exp.f32(i32 0, i32 15, float %arg0, float %arg0, float %arg0, float %psinput1, i1 false, i1 false) #0
   ret void
 }
 
+define amdgpu_ps float @vgpr_return(i32 %vgpr) {
+  ; CHECK-LABEL: name: vgpr_return
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   liveins: $vgpr0
+  ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
+  ; CHECK:   $vgpr0 = COPY [[COPY]](s32)
+  ; CHECK:   SI_RETURN_TO_EPILOG implicit $vgpr0
+  %cast = bitcast i32 %vgpr to float
+  ret float %cast
+}
+
+define amdgpu_ps i32 @sgpr_return_i32(i32 %vgpr) {
+  ; CHECK-LABEL: name: sgpr_return_i32
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   liveins: $vgpr0
+  ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
+  ; CHECK:   [[INT:%[0-9]+]]:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.readfirstlane), [[COPY]](s32)
+  ; CHECK:   $sgpr0 = COPY [[INT]](s32)
+  ; CHECK:   SI_RETURN_TO_EPILOG implicit $sgpr0
+  ret i32 %vgpr
+}
+
+define amdgpu_ps i64 @sgpr_return_i64(i64 %vgpr) {
+  ; CHECK-LABEL: name: sgpr_return_i64
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   liveins: $vgpr0, $vgpr1
+  ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
+  ; CHECK:   [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
+  ; CHECK:   [[MV:%[0-9]+]]:_(s64) = G_MERGE_VALUES [[COPY]](s32), [[COPY1]](s32)
+  ; CHECK:   [[UV:%[0-9]+]]:_(s32), [[UV1:%[0-9]+]]:_(s32) = G_UNMERGE_VALUES [[MV]](s64)
+  ; CHECK:   [[INT:%[0-9]+]]:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.readfirstlane), [[UV]](s32)
+  ; CHECK:   $sgpr0 = COPY [[INT]](s32)
+  ; CHECK:   [[INT1:%[0-9]+]]:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.readfirstlane), [[UV1]](s32)
+  ; CHECK:   $sgpr1 = COPY [[INT1]](s32)
+  ; CHECK:   SI_RETURN_TO_EPILOG implicit $sgpr0, implicit $sgpr1
+  ret i64 %vgpr
+}
+
+define amdgpu_ps <2 x i32> @sgpr_return_v2i32(<2 x i32> %vgpr) {
+  ; CHECK-LABEL: name: sgpr_return_v2i32
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   liveins: $vgpr0, $vgpr1
+  ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
+  ; CHECK:   [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
+  ; CHECK:   [[BUILD_VECTOR:%[0-9]+]]:_(<2 x s32>) = G_BUILD_VECTOR [[COPY]](s32), [[COPY1]](s32)
+  ; CHECK:   [[UV:%[0-9]+]]:_(s32), [[UV1:%[0-9]+]]:_(s32) = G_UNMERGE_VALUES [[BUILD_VECTOR]](<2 x s32>)
+  ; CHECK:   [[INT:%[0-9]+]]:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.readfirstlane), [[UV]](s32)
+  ; CHECK:   $sgpr0 = COPY [[INT]](s32)
+  ; CHECK:   [[INT1:%[0-9]+]]:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.readfirstlane), [[UV1]](s32)
+  ; CHECK:   $sgpr1 = COPY [[INT1]](s32)
+  ; CHECK:   SI_RETURN_TO_EPILOG implicit $sgpr0, implicit $sgpr1
+  ret <2 x i32> %vgpr
+}
+
+define amdgpu_ps { i32, i32 } @sgpr_struct_return_i32_i32(i32 %vgpr0, i32 %vgpr1) {
+  ; CHECK-LABEL: name: sgpr_struct_return_i32_i32
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   liveins: $vgpr0, $vgpr1
+  ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
+  ; CHECK:   [[COPY1:%[0-9]+]]:_(s32) = COPY $vgpr1
+  ; CHECK:   [[DEF:%[0-9]+]]:_(s32) = G_IMPLICIT_DEF
+  ; CHECK:   [[INT:%[0-9]+]]:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.readfirstlane), [[COPY]](s32)
+  ; CHECK:   $sgpr0 = COPY [[INT]](s32)
+  ; CHECK:   [[INT1:%[0-9]+]]:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.readfirstlane), [[COPY1]](s32)
+  ; CHECK:   $sgpr1 = COPY [[INT1]](s32)
+  ; CHECK:   SI_RETURN_TO_EPILOG implicit $sgpr0, implicit $sgpr1
+  %insertvalue0 = insertvalue { i32, i32 } undef, i32 %vgpr0, 0
+  %value = insertvalue { i32, i32 } %insertvalue0, i32 %vgpr1, 1
+  ret { i32, i32 } %value
+}
+
 declare void @llvm.amdgcn.exp.f32(i32 immarg, i32 immarg, float, float, float, float, i1 immarg, i1 immarg)  #0
 
 attributes #0 = { nounwind }
 attributes #1 = { "InitialPSInputAddr"="0x00002" }