[AMDGPU][GlobalISel] Legalize G_ABS

[mbrkusanin]

Summary

Legalize and select G_ABS so that we can use llvm.abs intrinsic

[foad]

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

2008

This has to be a signext, so the wide value gets the same sign as the narrow value. Otherwise the wide G_ABS might behave differently from the narrow G_ABS.

llvm/lib/Target/AMDGPU/VOP3Instructions.td

773 ↗

(On Diff #345075)

Don't bother with this pattern, just make G_ABS s16 only legal on subtargets that have 16 bit instructions.

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

11

Typo "sgpr" in lots of function names.

[foad]

llvm/lib/Target/AMDGPU/SOPInstructions.td

1370

No need for a separate pattern for this, you can do it as part of the definition of S_ABS_I32. See S_NOT_I32 for an example.

[arsenm]

llvm/lib/Target/AMDGPU/VOP3Instructions.td

758–760 ↗

(On Diff #345075)

Should be expanding the vector case in regbankselect

[mbrkusanin]

• Addressed comments

[foad]

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

13

This should be widened so we get s_sext followed by s_abs.

68

It would be simpler to widen this case to 32 bits, so we get the bfe followed by the usual 32-bit sub, max sequence.

[foad]

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

42–44

Not really related to your patch, but this sequence needs cleaning up. The s_and is redundant, and the s_cmp just computes the same scc value that s_add computed in the first place.

[arsenm]

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

42–44

This is probably a consequence of not having a pass to minimize scc/physreg liveranges

[mbrkusanin]

• Widen S16 to S32 for GFX6 + GFX7

[mbrkusanin]

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

42–44

We have this tracked as a separate issue.

[foad]

llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp

977

Merge this with the G_SMIN, G_SMAX, G_UMIN, G_UMAX handling above, which is slightly better because it will widen e.g. s8 to s16 and s24 to s32, instead of lowering them.

1000

Same here, mege it with the slightly better G_SMIN, G_SMAX, G_UMIN, G_UMAX handling above.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp

2320–2322

I would hope that you could call the LegalizerHelper for this part. However I see the lower() implementation hardcodes one path and doesn't provide the helpers for the multiple options. Ideally we could clean that up

[mbrkusanin]

• Merged G_ABS with G_SMIN, G_SMAX, G_UMIN, G_UMAX in LegalizerInfo

[foad]

LGTM, but maybe try Matt's idea about using LegalizerHelper functions as a follow up?

[mbrkusanin]

• Added legalizeABS() as a custom lowering to be used by RegBankSelect.

I don't see a way to add multiple paths to already existing lower() for G_ABS and control which one to use. Not sure if this is the cleaner way of doing it that you had in mind.

[mbrkusanin]

• Add lowerAbsToMaxSub as a new LegalizerHelper function to be used by AMDGPURegisterBankInfo
• Move default code for lowering G_ABS to lowerAbs to make it more clear there are two ways of legalizing.

Maybe rename lowerAbs to lowerAbsToAshrAddXor or lowerAbsToAddXor?

[foad]

LGTM.

Maybe rename lowerAbs to lowerAbsToAshrAddXor or lowerAbsToAddXor?

Maybe call them lowerAbsToAddXor and lowerAbsToMaxNeg?

llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp

2316

Too many parentheses.

[mbrkusanin]

• Suggested renaming
• Removed extra parentheses

llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp

LegalizerHelper::widenScalar(MachineInstr &MI, unsigned TypeIdx, LLT WideTy) {
   }
   case TargetOpcode::G_FREEZE:
     Observer.changingInstr(MI);
     widenScalarSrc(MI, WideTy, 1, TargetOpcode::G_ANYEXT);
     widenScalarDst(MI, WideTy);
     Observer.changedInstr(MI);
     return Legalized;
 
+  case TargetOpcode::G_ABS:
+    Observer.changingInstr(MI);
+    widenScalarSrc(MI, WideTy, 1, TargetOpcode::G_SEXT);
+    widenScalarDst(MI, WideTy);
+    Observer.changedInstr(MI);
+    return Legalized;
+
   case TargetOpcode::G_ADD:
   case TargetOpcode::G_AND:
   case TargetOpcode::G_MUL:
   case TargetOpcode::G_OR:
   case TargetOpcode::G_XOR:
   case TargetOpcode::G_SUB:
     // Perform operation at larger width (any extension is fines here, high bits
     // don't affect the result) and then truncate the result back to the
LegalizerHelper::fewerElementsVector(MachineInstr &MI, unsigned TypeIdx,
   case G_OR:
   case G_XOR:
   case G_ADD:
   case G_SUB:
   case G_MUL:
   case G_PTR_ADD:
   case G_SMULH:
   case G_UMULH:
+  case G_ABS:
   case G_FADD:
   case G_FMUL:
   case G_FSUB:
   case G_FNEG:
   case G_FABS:
   case G_FCANONICALIZE:
   case G_FDIV:
   case G_FREM:

llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp

if (ST.has16BitInsts()) {
     } else {
       getActionDefinitionsBuilder({G_SMIN, G_SMAX, G_UMIN, G_UMAX})
         .legalFor({S32, S16})
         .widenScalarToNextPow2(0)
         .minScalar(0, S16)
         .scalarize(0)
         .lower();
     }
+
+    getActionDefinitionsBuilder(G_ABS)
+      .legalFor({S32, S16})
+      .scalarize(0)
+      .lower();
   } else {
     // TODO: Should have same legality without v_perm_b32
     getActionDefinitionsBuilder(G_BSWAP)
       .legalFor({S32})
       .lowerIf(scalarNarrowerThan(0, 32))
       // FIXME: Fixing non-power-of-2 before clamp is workaround for
       // narrowScalar limitation.
       .widenScalarToNextPow2(0)
       .maxScalar(0, S32)
       .scalarize(0)
       .lower();
 
     getActionDefinitionsBuilder({G_SMIN, G_SMAX, G_UMIN, G_UMAX})
       .legalFor({S32})
       .minScalar(0, S32)
       .widenScalarToNextPow2(0)
       .scalarize(0)
       .lower();
+
+    getActionDefinitionsBuilder(G_ABS)
+      .legalFor({S32})
+      .scalarize(0)
+      .lower();
   }
 
   getActionDefinitionsBuilder(G_INTTOPTR)
     // List the common cases
     .legalForCartesianProduct(AddrSpaces64, {S64})
     .legalForCartesianProduct(AddrSpaces32, {S32})
     .scalarize(0)
     // Accept any address space as long as the size matches

llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp

case AMDGPU::G_XOR: {
 
     B.buildInstr(Opc, {DefRegs[0]}, {Src0Regs[0], Src1Regs[0]});
     B.buildInstr(Opc, {DefRegs[1]}, {Src0Regs[1], Src1Regs[1]});
 
     MRI.setRegBank(DstReg, AMDGPU::VGPRRegBank);
     MI.eraseFromParent();
     return;
   }
+  case AMDGPU::G_ABS: {
+    Register SrcReg = MI.getOperand(1).getReg();
+    const RegisterBank *SrcBank = MRI.getRegBankOrNull(SrcReg);
+
+    // There is no VALU abs instruction so we need to replace it with a sub and
+    // max combination.
+    if (SrcBank && SrcBank == &AMDGPU::VGPRRegBank) {
+      MachineIRBuilder B(MI);
+      LLT Ty = MRI.getType(SrcReg);
+
+      Register Zero = B.buildConstant(Ty, 0).getReg(0);
+      Register Sub = B.buildSub(Ty, Zero, SrcReg).getReg(0);
+      B.buildSMax(MI.getOperand(0), SrcReg, Sub);
+
+      MRI.setRegBank(Zero, AMDGPU::SGPRRegBank);
+      MRI.setRegBank(Sub, AMDGPU::VGPRRegBank);
+
+      MI.eraseFromParent();
+      return;
+    }
+    LLVM_FALLTHROUGH;
+  }
   case AMDGPU::G_ADD:
   case AMDGPU::G_SUB:
   case AMDGPU::G_MUL:
   case AMDGPU::G_SHL:
   case AMDGPU::G_LSHR:
   case AMDGPU::G_ASHR:
   case AMDGPU::G_SMIN:
   case AMDGPU::G_SMAX:
if (Size == 64) {
 
       break;
     }
 
     LLVM_FALLTHROUGH;
   }
   case AMDGPU::G_PTR_ADD:
   case AMDGPU::G_PTRMASK:
+  case AMDGPU::G_ABS:
   case AMDGPU::G_ADD:
   case AMDGPU::G_SUB:
   case AMDGPU::G_MUL:
   case AMDGPU::G_SHL:
   case AMDGPU::G_LSHR:
   case AMDGPU::G_ASHR:
   case AMDGPU::G_UADDO:
   case AMDGPU::G_USUBO:

llvm/lib/Target/AMDGPU/SOPInstructions.td

 def S_MOVRELD_B64 : SOP1_64_movreld <"s_movreld_b64">;
 } // End Uses = [M0]
 
 let SubtargetPredicate = isGFX6GFX7GFX8GFX9 in {
 def S_CBRANCH_JOIN : SOP1_0_32R <"s_cbranch_join">;
 } // End SubtargetPredicate = isGFX6GFX7GFX8GFX9
 
 let Defs = [SCC] in {
-def S_ABS_I32 : SOP1_32 <"s_abs_i32">;
+def S_ABS_I32 : SOP1_32 <"s_abs_i32",
+    [(set i32:$sdst, (abs i32:$src0))]
+  >;
 } // End Defs = [SCC]
 
 let SubtargetPredicate = HasVGPRIndexMode in {
 def S_SET_GPR_IDX_IDX : SOP1_0_32<"s_set_gpr_idx_idx"> {
   let Uses = [M0, MODE];
   let Defs = [M0, MODE];
 }
 }

 >;
 
 // Same as a 32-bit inreg
 def : GCNPat<
   (i32 (UniformUnaryFrag<sext> i16:$src)),
   (S_SEXT_I32_I16 $src)
 >;
 
 
 //===----------------------------------------------------------------------===//
 // SOP2 Patterns
 //===----------------------------------------------------------------------===//
 
 // V_ADD_I32_e32/S_ADD_U32 produces carry in VCC/SCC. For the vector
 // case, the sgpr-copies pass will fix this to use the vector version.
 def : GCNPat <
   (i32 (addc i32:$src0, i32:$src1)),

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

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -global-isel -march=amdgcn -mcpu=tahiti -verify-machineinstrs -o - < %s | FileCheck %s --check-prefixes=GFX,GFX6
+; RUN: llc -global-isel -march=amdgcn -mcpu=fiji -verify-machineinstrs -o - < %s | FileCheck %s --check-prefixes=GFX,GFX8
+
+declare i16 @llvm.abs.i16(i16, i1)
+declare i32 @llvm.abs.i32(i32, i1)
+declare i64 @llvm.abs.i64(i64, i1)
+declare <4 x i32> @llvm.abs.v4i32(<4 x i32>, i1)
+
+define amdgpu_cs i16 @abs_sgpr_i16(i16 inreg %arg) {
+; GFX6-LABEL: abs_sgpr_i16:
+; GFX6:       ; %bb.0:
+; GFX6-NEXT:    s_sext_i32_i16 s1, s0
+; GFX6-NEXT:    s_ashr_i32 s1, s1, 15
+; GFX6-NEXT:    s_add_i32 s0, s0, s1
+; GFX6-NEXT:    s_xor_b32 s0, s0, s1
+; GFX6-NEXT:    ; return to shader part epilog
+;
+; GFX8-LABEL: abs_sgpr_i16:
+; GFX8:       ; %bb.0:
+; GFX8-NEXT:    s_sext_i32_i16 s0, s0
+; GFX8-NEXT:    s_abs_i32 s0, s0
+; GFX8-NEXT:    ; return to shader part epilog
+  %res = call i16 @llvm.abs.i16(i16 %arg, i1 false)
+  ret i16 %res
+}
+
+define amdgpu_cs i32 @abs_sgpr_i32(i32 inreg %arg) {
+; GFX-LABEL: abs_sgpr_i32:
+; GFX:       ; %bb.0:
+; GFX-NEXT:    s_abs_i32 s0, s0
+; GFX-NEXT:    ; return to shader part epilog
+  %res = call i32 @llvm.abs.i32(i32 %arg, i1 false)
+  ret i32 %res
+}
+
+define amdgpu_cs i64 @abs_sgpr_i64(i64 inreg %arg) {
+; GFX-LABEL: abs_sgpr_i64:
+; GFX:       ; %bb.0:
+; GFX-NEXT:    s_ashr_i32 s2, s1, 31
+; GFX-NEXT:    s_add_u32 s0, s0, s2
+; GFX-NEXT:    s_cselect_b32 s4, 1, 0
+; GFX-NEXT:    s_and_b32 s4, s4, 1
+; GFX-NEXT:    s_cmp_lg_u32 s4, 0
+; GFX-NEXT:    s_mov_b32 s3, s2
+; GFX-NEXT:    s_addc_u32 s1, s1, s2
+; GFX-NEXT:    s_xor_b64 s[0:1], s[0:1], s[2:3]
+; GFX-NEXT:    ; return to shader part epilog
+  %res = call i64 @llvm.abs.i64(i64 %arg, i1 false)
+  ret i64 %res
+}
+
+define amdgpu_cs <4 x i32> @abs_sgpr_v4i32(<4 x i32> inreg %arg) {
+; GFX-LABEL: abs_sgpr_v4i32:
+; GFX:       ; %bb.0:
+; GFX-NEXT:    s_abs_i32 s0, s0
+; GFX-NEXT:    s_abs_i32 s1, s1
+; GFX-NEXT:    s_abs_i32 s2, s2
+; GFX-NEXT:    s_abs_i32 s3, s3
+; GFX-NEXT:    ; return to shader part epilog
+  %res = call <4 x i32> @llvm.abs.v4i32(<4 x i32> %arg, i1 false)
+  ret <4 x i32> %res
+}
+
+define amdgpu_cs i16 @abs_vgpr_i16(i16 %arg) {
+; GFX6-LABEL: abs_vgpr_i16:
+; GFX6:       ; %bb.0:
+; GFX6-NEXT:    v_bfe_i32 v1, v0, 0, 16
+; GFX6-NEXT:    v_ashrrev_i32_e32 v1, 15, v1
+; GFX6-NEXT:    v_add_i32_e32 v0, vcc, v0, v1
+; GFX6-NEXT:    v_xor_b32_e32 v0, v0, v1
+; GFX6-NEXT:    v_readfirstlane_b32 s0, v0
+; GFX6-NEXT:    ; return to shader part epilog
+;
+; GFX8-LABEL: abs_vgpr_i16:
+; GFX8:       ; %bb.0:
+; GFX8-NEXT:    v_sub_u16_e32 v1, 0, v0
+; GFX8-NEXT:    v_max_i16_e32 v0, v0, v1
+; GFX8-NEXT:    v_readfirstlane_b32 s0, v0
+; GFX8-NEXT:    ; return to shader part epilog
+  %res = call i16 @llvm.abs.i16(i16 %arg, i1 false)
+  ret i16 %res
+}
+
+define amdgpu_cs i32 @abs_vgpr_i32(i32 %arg) {
+; GFX6-LABEL: abs_vgpr_i32:
+; GFX6:       ; %bb.0:
+; GFX6-NEXT:    v_sub_i32_e32 v1, vcc, 0, v0
+; GFX6-NEXT:    v_max_i32_e32 v0, v0, v1
+; GFX6-NEXT:    v_readfirstlane_b32 s0, v0
+; GFX6-NEXT:    ; return to shader part epilog
+;
+; GFX8-LABEL: abs_vgpr_i32:
+; GFX8:       ; %bb.0:
+; GFX8-NEXT:    v_sub_u32_e32 v1, vcc, 0, v0
+; GFX8-NEXT:    v_max_i32_e32 v0, v0, v1
+; GFX8-NEXT:    v_readfirstlane_b32 s0, v0
+; GFX8-NEXT:    ; return to shader part epilog
+  %res = call i32 @llvm.abs.i32(i32 %arg, i1 false)
+  ret i32 %res
+}
+
+define amdgpu_cs i64 @abs_vgpr_i64(i64 %arg) {
+; GFX6-LABEL: abs_vgpr_i64:
+; GFX6:       ; %bb.0:
+; GFX6-NEXT:    v_ashrrev_i32_e32 v2, 31, v1
+; GFX6-NEXT:    v_add_i32_e32 v0, vcc, v0, v2
+; GFX6-NEXT:    v_addc_u32_e32 v1, vcc, v1, v2, vcc
+; GFX6-NEXT:    v_xor_b32_e32 v0, v0, v2
+; GFX6-NEXT:    v_xor_b32_e32 v1, v1, v2
+; GFX6-NEXT:    v_readfirstlane_b32 s0, v0
+; GFX6-NEXT:    v_readfirstlane_b32 s1, v1
+; GFX6-NEXT:    ; return to shader part epilog
+;
+; GFX8-LABEL: abs_vgpr_i64:
+; GFX8:       ; %bb.0:
+; GFX8-NEXT:    v_ashrrev_i32_e32 v2, 31, v1
+; GFX8-NEXT:    v_add_u32_e32 v0, vcc, v0, v2
+; GFX8-NEXT:    v_addc_u32_e32 v1, vcc, v1, v2, vcc
+; GFX8-NEXT:    v_xor_b32_e32 v0, v0, v2
+; GFX8-NEXT:    v_xor_b32_e32 v1, v1, v2
+; GFX8-NEXT:    v_readfirstlane_b32 s0, v0
+; GFX8-NEXT:    v_readfirstlane_b32 s1, v1
+; GFX8-NEXT:    ; return to shader part epilog
+  %res = call i64 @llvm.abs.i64(i64 %arg, i1 false)
+  ret i64 %res
+}
+
+define amdgpu_cs <4 x i32> @abs_vgpr_v4i32(<4 x i32> %arg) {
+; GFX6-LABEL: abs_vgpr_v4i32:
+; GFX6:       ; %bb.0:
+; GFX6-NEXT:    v_sub_i32_e32 v4, vcc, 0, v0
+; GFX6-NEXT:    v_max_i32_e32 v0, v0, v4
+; GFX6-NEXT:    v_sub_i32_e32 v4, vcc, 0, v1
+; GFX6-NEXT:    v_max_i32_e32 v1, v1, v4
+; GFX6-NEXT:    v_sub_i32_e32 v4, vcc, 0, v2
+; GFX6-NEXT:    v_max_i32_e32 v2, v2, v4
+; GFX6-NEXT:    v_sub_i32_e32 v4, vcc, 0, v3
+; GFX6-NEXT:    v_max_i32_e32 v3, v3, v4
+; GFX6-NEXT:    v_readfirstlane_b32 s0, v0
+; GFX6-NEXT:    v_readfirstlane_b32 s1, v1
+; GFX6-NEXT:    v_readfirstlane_b32 s2, v2
+; GFX6-NEXT:    v_readfirstlane_b32 s3, v3
+; GFX6-NEXT:    ; return to shader part epilog
+;
+; GFX8-LABEL: abs_vgpr_v4i32:
+; GFX8:       ; %bb.0:
+; GFX8-NEXT:    v_sub_u32_e32 v4, vcc, 0, v0
+; GFX8-NEXT:    v_max_i32_e32 v0, v0, v4
+; GFX8-NEXT:    v_sub_u32_e32 v4, vcc, 0, v1
+; GFX8-NEXT:    v_max_i32_e32 v1, v1, v4
+; GFX8-NEXT:    v_sub_u32_e32 v4, vcc, 0, v2
+; GFX8-NEXT:    v_max_i32_e32 v2, v2, v4
+; GFX8-NEXT:    v_sub_u32_e32 v4, vcc, 0, v3
+; GFX8-NEXT:    v_max_i32_e32 v3, v3, v4
+; GFX8-NEXT:    v_readfirstlane_b32 s0, v0
+; GFX8-NEXT:    v_readfirstlane_b32 s1, v1
+; GFX8-NEXT:    v_readfirstlane_b32 s2, v2
+; GFX8-NEXT:    v_readfirstlane_b32 s3, v3
+; GFX8-NEXT:    ; return to shader part epilog
+  %res = call <4 x i32> @llvm.abs.v4i32(<4 x i32> %arg, i1 false)
+  ret <4 x i32> %res
+}