[AMDGPU][GlobalISel] Avoid selecting S_PACK with constants

[mbrkusanin]

Summary

If constants are hidden behind G_ANYEXT we can treat them same way as G_SEXT.
For that purpose we extend getConstantVRegValWithLookThrough with option
to handle G_ANYEXT same way as G_SEXT.

[foad]

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

980–988

I would prefer not to add more ad hoc constant folding to this pass. Can you work out where the un-folded pack instructions are coming from in the first place, and fix it there instead?

[mbrkusanin]

Specific case from one of the tests:

%5:sgpr(s16) = G_FCONSTANT half 0xH4000
%7:sgpr(s32) = G_ANYEXT %5:sgpr(s16)
%4:sreg_32(<2 x s16>) = G_BUILD_VECTOR_TRUNC %7:sgpr(s32), %7:sgpr(s32)

will now be:

%4:sreg_32 = S_MOV_B32 1073758208

instead of:

%5:sreg_32 = S_MOV_B32 16384
%4:sreg_32 = S_PACK_LL_B32_B16 %5:sreg_32, %5:sreg_32

[foad]

llvm/include/llvm/CodeGen/GlobalISel/Utils.h

146 ↗

(On Diff #308027)

I'm not sure if we need a flag for this. Could we do it unconditionally? @qcolombet ?

[mbrkusanin]

llvm/include/llvm/CodeGen/GlobalISel/Utils.h

146 ↗

(On Diff #308027)

There were no regressions for check-all when I tried it that way, but I wasn't sure if I should leave it like that.

[foad]

llvm/include/llvm/CodeGen/GlobalISel/Utils.h

146 ↗

(On Diff #308027)

I think it's probably safe to treat anyext the same as zext in this function, but maybe wait for comments from other globalisel maintainers (who may be on thanksgiving vacation).

[foad]

Specific case from one of the tests:

%5:sgpr(s16) = G_FCONSTANT half 0xH4000
%7:sgpr(s32) = G_ANYEXT %5:sgpr(s16)
%4:sreg_32(<2 x s16>) = G_BUILD_VECTOR_TRUNC %7:sgpr(s32), %7:sgpr(s32)

Perhaps there should be a combine that simplifies this to G_BUILD_VECTOR %5, %5.

[arsenm]

In D92219#2420126, @foad wrote:

Specific case from one of the tests:

%5:sgpr(s16) = G_FCONSTANT half 0xH4000
%7:sgpr(s32) = G_ANYEXT %5:sgpr(s16)
%4:sreg_32(<2 x s16>) = G_BUILD_VECTOR_TRUNC %7:sgpr(s32), %7:sgpr(s32)

Perhaps there should be a combine that simplifies this to G_BUILD_VECTOR %5, %5.

This is specifically avoiding 16-bit inputs, we expanded the other direction. It could be a bitcast of a scalar constant. I'm trying a somewhat different strategy for 16-bit vectors than the DAG but not sure what the end state is going to look like

[arsenm]

llvm/lib/CodeGen/GlobalISel/Utils.cpp

298–301 ↗

(On Diff #308027)

I considered doing this in D84165 but then ran into some case that made me hesitate but I don't remember the details

[foad]

I think this looks good, and we could even get rid of the AnyExtAsZext flag and do it unconditionally. Adding some globalisel folks to see if there are any objections.

[paquette]

llvm/include/llvm/CodeGen/GlobalISel/Utils.h

146 ↗

(On Diff #308027)

Maybe this should be LookThroughAnyExt? I imagine there are situations where we might want to treat G_ANYEXT as a sign extend.

e.g.

%cst:_(s32) = G_CONSTANT i32 -1
%cst_wide:_(s64) = G_ANYEXT %cst
...
%y:_(s64) = ...
%x:_(s64) = G_ADD %y, %cst_wide

[foad]

llvm/include/llvm/CodeGen/GlobalISel/Utils.h

146 ↗

(On Diff #308027)

You mean, change the name of the argument to LookThroughAnyExt? But what would it do? Currently this function has to either sign- or zero-extend whatever constant it finds, so that it can return a value of the right bit width.

[mbrkusanin]

Rename + G_ANYEXT will now be treated same as G_SEXT instead of G_ZEXT.

[foad]

LGTM.

[dsanders]

llvm/include/llvm/CodeGen/GlobalISel/Utils.h

146 ↗

(On Diff #308027)

Currently this function has to either sign- or zero-extend whatever constant it finds

I think Jessica was pointing out that which extend (if any) is the correct thing to do is context and target dependent.

To add to the target dependent side of that: G_ANYEXT is an extension with undefined bits and you're allowed to pick zero/sign-extend if that's convenient for your target but different targets will prefer sign or zero extend and some won't care either way (e.g. because they can operate on the smaller bitwidth). In the case of Mips, it's potentially width dependent due to some quirks of forward compatibility on that target where registers are theoretically sign extended to infinite width even if the value is supposed to be zero extended.

llvm/lib/Target/AMDGPU/SIFoldOperands.cpp

static bool evalBinaryInstruction(unsigned Opcode, int32_t &Result,
   case AMDGPU::V_ASHR_I32_e32:
   case AMDGPU::S_ASHR_I32:
     Result = static_cast<int32_t>(LHS) >> (RHS & 31);
     return true;
   case AMDGPU::V_ASHRREV_I32_e64:
   case AMDGPU::V_ASHRREV_I32_e32:
     Result = static_cast<int32_t>(RHS) >> (LHS & 31);
     return true;
+  case AMDGPU::S_PACK_LL_B32_B16:
+    Result = (LHS & 0xffff) | ((RHS & 0xffff) << 16);
+    return true;
+  case AMDGPU::S_PACK_LH_B32_B16:
+    Result = (LHS & 0xffff) | (RHS & 0xffff0000);
+    return true;
+  case AMDGPU::S_PACK_HH_B32_B16:
+    Result = ((LHS & 0xffff0000) >> 16) | (RHS & 0xffff0000);
+    return true;
   default:
     return false;
   }
 }
 
 static unsigned getMovOpc(bool IsScalar) {
   return IsScalar ? AMDGPU::S_MOV_B32 : AMDGPU::V_MOV_B32_e32;
 }

llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.fdot2.ll

; GFX10-NEXT:    s_setpc_b64 s[30:31]
   %r = call float @llvm.amdgcn.fdot2(<2 x half> %a, <2 x half> %b, float %neg.c, i1 false)
   ret float %r
 }
 
 define float @v_fdot2_inline_literal_a(<2 x half> %b, float %c) {
 ; GFX906-LABEL: v_fdot2_inline_literal_a:
 ; GFX906:       ; %bb.0:
 ; GFX906-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
-; GFX906-NEXT:    s_movk_i32 s4, 0x4000
-; GFX906-NEXT:    s_pack_ll_b32_b16 s4, s4, s4
-; GFX906-NEXT:    v_dot2_f32_f16 v0, s4, v0, v1
+; GFX906-NEXT:    v_dot2_f32_f16 v0, 2.0, v0, v1 op_sel_hi:[0,1,1]
 ; GFX906-NEXT:    s_setpc_b64 s[30:31]
 ;
 ; GFX10-LABEL: v_fdot2_inline_literal_a:
 ; GFX10:       ; %bb.0:
 ; GFX10-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX10-NEXT:    s_waitcnt_vscnt null, 0x0
-; GFX10-NEXT:    s_movk_i32 s4, 0x4000
+; GFX10-NEXT:    v_dot2_f32_f16 v0, 2.0, v0, v1 op_sel_hi:[0,1,1]
 ; GFX10-NEXT:    ; implicit-def: $vcc_hi
-; GFX10-NEXT:    s_pack_ll_b32_b16 s4, s4, s4
-; GFX10-NEXT:    v_dot2_f32_f16 v0, s4, v0, v1
 ; GFX10-NEXT:    s_setpc_b64 s[30:31]
   %ret = tail call float @llvm.amdgcn.fdot2(<2 x half> <half 2.0, half 2.0>, <2 x half> %b, float %c, i1 false)
   ret float %ret
 }
 
 define float @v_fdot2_inline_literal_b(<2 x half> %a, float %c) {
 ; GFX906-LABEL: v_fdot2_inline_literal_b:
 ; GFX906:       ; %bb.0:
 ; GFX906-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
-; GFX906-NEXT:    s_movk_i32 s4, 0x4000
-; GFX906-NEXT:    s_pack_ll_b32_b16 s4, s4, s4
-; GFX906-NEXT:    v_dot2_f32_f16 v0, v0, s4, v1
+; GFX906-NEXT:    v_dot2_f32_f16 v0, v0, 2.0, v1 op_sel_hi:[1,0,1]
 ; GFX906-NEXT:    s_setpc_b64 s[30:31]
 ;
 ; GFX10-LABEL: v_fdot2_inline_literal_b:
 ; GFX10:       ; %bb.0:
 ; GFX10-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
 ; GFX10-NEXT:    s_waitcnt_vscnt null, 0x0
-; GFX10-NEXT:    s_movk_i32 s4, 0x4000
+; GFX10-NEXT:    v_dot2_f32_f16 v0, v0, 2.0, v1 op_sel_hi:[1,0,1]
 ; GFX10-NEXT:    ; implicit-def: $vcc_hi
-; GFX10-NEXT:    s_pack_ll_b32_b16 s4, s4, s4
-; GFX10-NEXT:    v_dot2_f32_f16 v0, v0, s4, v1
 ; GFX10-NEXT:    s_setpc_b64 s[30:31]
   %ret = tail call float @llvm.amdgcn.fdot2(<2 x half> %a, <2 x half> <half 2.0, half 2.0>, float %c, i1 false)
   ret float %ret
 }
 
 define float @v_fdot2_inline_literal_c(<2 x half> %a, <2 x half> %b) {
 ; GFX906-LABEL: v_fdot2_inline_literal_c:
 ; GFX906:       ; %bb.0:

llvm/test/CodeGen/AMDGPU/constant-fold-imm-immreg.mir

bb.0:
     %0:vgpr_32 = COPY $vgpr0
     %1:vgpr_32 = V_MOV_B32_e32 16, implicit $exec
     %2:vgpr_32 = V_MOV_B32_sdwa 0, %0:vgpr_32, 0, 5, 2, 4, implicit $exec, implicit %0:vgpr_32(tied-def 0)
     %3:vgpr_32 = V_MOV_B32_e32 16, implicit $exec
     %4:vgpr_32 = V_LSHRREV_B32_e64 %3:vgpr_32, %2:vgpr_32, implicit $exec
     S_ENDPGM 0, implicit %4
 
 ...
+
+---
+name:            fold_pack_ll_imm_imm
+alignment:       0
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+
+body:             |
+  bb.0:
+    ;liveins: $vgpr0
+
+    ; GCN-LABEL: name: fold_pack_ll_imm_imm
+    ; GCN: [[S_MOV_B32_:%[0-9]+]]:sreg_32 = S_MOV_B32 196609
+    ; GCN: [[S_MOV_B32_1:%[0-9]+]]:sreg_32 = S_MOV_B32 65539
+    ; GCN: $sgpr0 = COPY [[S_MOV_B32_]]
+    ; GCN: $sgpr1 = COPY [[S_MOV_B32_1]]
+    ; GCN: SI_RETURN_TO_EPILOG implicit $sgpr0, implicit $sgpr1
+    %1:sreg_32 = S_MOV_B32 1
+    %2:sreg_32 = S_MOV_B32 3
+    %3:sreg_32 = S_PACK_LL_B32_B16 %1, %2
+    %4:sreg_32 = S_PACK_LL_B32_B16 %2, %1
+    $sgpr0 = COPY %3:sreg_32
+    $sgpr1 = COPY %4:sreg_32
+    SI_RETURN_TO_EPILOG implicit $sgpr0, implicit $sgpr1
+
+...
+
+---
+name:            fold_pack_lh_imm_imm
+alignment:       0
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+
+body:             |
+  bb.0:
+    ;liveins: $vgpr0
+
+    ; GCN-LABEL: name: fold_pack_lh_imm_imm
+    ; GCN: [[S_MOV_B32_:%[0-9]+]]:sreg_32 = S_MOV_B32 65537
+    ; GCN: [[S_MOV_B32_1:%[0-9]+]]:sreg_32 = S_MOV_B32 0
+    ; GCN: $sgpr0 = COPY [[S_MOV_B32_]]
+    ; GCN: $sgpr1 = COPY [[S_MOV_B32_1]]
+    ; GCN: SI_RETURN_TO_EPILOG implicit $sgpr0, implicit $sgpr1
+    %1:sreg_32 = S_MOV_B32 1
+    %2:sreg_32 = S_MOV_B32 65536
+    %3:sreg_32 = S_PACK_LH_B32_B16 %1, %2
+    %4:sreg_32 = S_PACK_LH_B32_B16 %2, %1
+    $sgpr0 = COPY %3:sreg_32
+    $sgpr1 = COPY %4:sreg_32
+    SI_RETURN_TO_EPILOG implicit $sgpr0, implicit $sgpr1
+
+...
+
+---
+name:            fold_pack_hh_imm_imm
+alignment:       0
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+
+body:             |
+  bb.0:
+    ;liveins: $vgpr0
+
+    ; GCN-LABEL: name: fold_pack_hh_imm_imm
+    ; GCN: [[S_MOV_B32_:%[0-9]+]]:sreg_32 = S_MOV_B32 65538
+    ; GCN: [[S_MOV_B32_1:%[0-9]+]]:sreg_32 = S_MOV_B32 131073
+    ; GCN: $sgpr0 = COPY [[S_MOV_B32_]]
+    ; GCN: $sgpr1 = COPY [[S_MOV_B32_1]]
+    ; GCN: SI_RETURN_TO_EPILOG implicit $sgpr0, implicit $sgpr1
+    %1:sreg_32 = S_MOV_B32 131072
+    %2:sreg_32 = S_MOV_B32 65536
+    %3:sreg_32 = S_PACK_HH_B32_B16 %1, %2
+    %4:sreg_32 = S_PACK_HH_B32_B16 %2, %1
+    $sgpr0 = COPY %3:sreg_32
+    $sgpr1 = COPY %4:sreg_32
+    SI_RETURN_TO_EPILOG implicit $sgpr0, implicit $sgpr1
+
+...