[GlobalISel] Avoid making G_PTR_ADD with nullptr

[mbrkusanin]

Summary

When the first operand is a null pointer we can avoid making a G_PTR_ADD and
make a G_INTTOPTR with the offset operand.
This helps us avoid making add with 0 later on for targets such as AMDGPU.

[arsenm]

This is probably wrong for non integral pointers. Why not do this as a combine later? We need to start recognizing G_PTRADD (constant) + constant anyway to handle lowered LDS addresses

[foad]

llvm/test/CodeGen/AArch64/GlobalISel/translate-gep.ll

11 ↗

(On Diff #289930)

Can you avoid generating this unused constant in the first place?

[mbrkusanin]

• Reimplemented as a AMDGPU GICombineRule.

[arsenm]

llvm/lib/Target/AMDGPU/AMDGPUCombine.td

40

ptr_add

40–45

This should go in the generic combiner

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

45

Should not construct a local MachineIRBuilder

llvm/test/CodeGen/AMDGPU/GlobalISel/combine-add-nullptr.mir

52–53

Can simplify these out of the test

56

Should add vector test

[arsenm]

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

2737–2740 ↗

(On Diff #294334)

Combine the scalar and vector check above?

2743 ↗

(On Diff #294334)

I have a patch to generalize these checks I haven't posted yet, but I guess having the two paths is fine for now

[mbrkusanin]

• Refactored matchPtrAddZero().

[mbrkusanin]

• Rebase
• Ping

[foad]

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

2774 ↗

(On Diff #297787)

This is just Ty.getScalarType().

2783 ↗

(On Diff #297787)

You should be able to assert that it is a vector here.

[mbrkusanin]

• Addressed comments.

[foad]

LGTM.

llvm/lib/Target/AMDGPU/AMDGPUCombine.td

def cvt_f32_ubyteN : GICombineRule<
   (defs root:$cvt_f32_ubyteN, cvt_f32_ubyteN_matchdata:$matchinfo),
   (match (wip_match_opcode G_AMDGPU_CVT_F32_UBYTE0,
                            G_AMDGPU_CVT_F32_UBYTE1,
                            G_AMDGPU_CVT_F32_UBYTE2,
                            G_AMDGPU_CVT_F32_UBYTE3):$cvt_f32_ubyteN,
          [{ return matchCvtF32UByteN(*${cvt_f32_ubyteN}, MRI, *MF, ${matchinfo}); }]),
   (apply [{ applyCvtF32UByteN(*${cvt_f32_ubyteN}, ${matchinfo}); }])>;
 
+def add_with_zero: GICombineRule<
+  (defs root:$root),
+  (match (wip_match_opcode G_PTR_ADD):$root,
+    [{ return matchAddZero(*${root}, MRI, *MF, Helper); }]),
+  (apply [{ applyAddZero(*${root}); }])>;
+
 // Combines which should only apply on SI/VI
 def gfx6gfx7_combines : GICombineGroup<[fcmp_select_to_fmin_fmax_legacy]>;
 
 
 def AMDGPUPreLegalizerCombinerHelper: GICombinerHelper<
-  "AMDGPUGenPreLegalizerCombinerHelper", [all_combines]> {
+  "AMDGPUGenPreLegalizerCombinerHelper", [all_combines, add_with_zero]> {
   let DisableRuleOption = "amdgpuprelegalizercombiner-disable-rule";
 }
 
 def AMDGPUPostLegalizerCombinerHelper: GICombinerHelper<
   "AMDGPUGenPostLegalizerCombinerHelper",
   [all_combines, gfx6gfx7_combines,
    uchar_to_float, cvt_f32_ubyteN]> {
   let DisableRuleOption = "amdgpupostlegalizercombiner-disable-rule";
 }
 
 def AMDGPURegBankCombinerHelper : GICombinerHelper<
   "AMDGPUGenRegBankCombinerHelper", []> {
   let DisableRuleOption = "amdgpuregbankcombiner-disable-rule";
 }

llvm/lib/Target/AMDGPU/AMDGPUPreLegalizerCombiner.cpp

 #include "llvm/Support/Debug.h"
 #include "MCTargetDesc/AMDGPUMCTargetDesc.h"
 
 #define DEBUG_TYPE "amdgpu-prelegalizer-combiner"
 
 using namespace llvm;
 using namespace MIPatternMatch;
 
+static bool matchAddZero(MachineInstr &MI, MachineRegisterInfo &MRI,
+                         MachineFunction &MF, CombinerHelper &Helper) {
+  Register DstReg = MI.getOperand(0).getReg();
+  LLT Ty = MRI.getType(DstReg);
+  const DataLayout &DL = MF.getDataLayout();
+
+  if (Ty.isPointer() && DL.isNonIntegralAddressSpace(Ty.getAddressSpace()))
+    return false;
+
+  auto ConstVal = getConstantVRegVal(MI.getOperand(1).getReg(), MRI);
+  return ConstVal && *ConstVal == 0;
+}
+
+static void applyAddZero(MachineInstr &MI) {
+  MachineIRBuilder B(MI);
+  B.buildIntToPtr(MI.getOperand(0), MI.getOperand(2));
+  MI.eraseFromParent();
+}
+
 #define AMDGPUPRELEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_DEPS
 #include "AMDGPUGenPreLegalizeGICombiner.inc"
 #undef AMDGPUPRELEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_DEPS
 
 namespace {
 #define AMDGPUPRELEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_H
 #include "AMDGPUGenPreLegalizeGICombiner.inc"
 #undef AMDGPUPRELEGALIZERCOMBINERHELPER_GENCOMBINERHELPER_H

llvm/test/CodeGen/AMDGPU/GlobalISel/combine-add-nullptr.mir

+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=amdgcn-amd-amdhsa -run-pass=amdgpu-prelegalizer-combiner -verify-machineinstrs %s -o - | FileCheck %s
+
+---
+name: add_nullptr_sgpr
+tracksRegLiveness: true
+body:             |
+  bb.0.:
+    liveins: $sgpr0, $sgpr1
+
+    ; CHECK-LABEL: name: add_nullptr_sgpr
+    ; CHECK: liveins: $sgpr0, $sgpr1
+    ; CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY $sgpr0
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 3
+    ; CHECK: [[SHL:%[0-9]+]]:_(s32) = G_SHL [[COPY]], [[C]](s32)
+    ; CHECK: [[INT:%[0-9]+]]:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.readfirstlane), [[SHL]](s32)
+    ; CHECK: $sgpr0 = COPY [[INT]](s32)
+    ; CHECK: SI_RETURN_TO_EPILOG implicit $sgpr0
+    %0:_(s32) = COPY $sgpr0
+    %10:_(s32) = G_CONSTANT i32 3
+    %5:_(s32) = G_SHL %0, %10(s32)
+    %3:_(p3) = G_CONSTANT i32 0
+    %6:_(p3) = G_PTR_ADD %3, %5(s32)
+    %8:_(s32) = G_PTRTOINT %6(p3)
+    %9:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.readfirstlane), %8(s32)
+    $sgpr0 = COPY %9(s32)
+    SI_RETURN_TO_EPILOG implicit $sgpr0
+
+...
+
+---
+name: add_nullptr_vgpr
+tracksRegLiveness: true
+body:             |
+  bb.0.:
+    liveins: $vgpr0, $vgpr1
+
+    ; CHECK-LABEL: name: add_nullptr_vgpr
+    ; CHECK: liveins: $vgpr0, $vgpr1
+    ; CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY $vgpr0
+    ; CHECK: [[C:%[0-9]+]]:_(s32) = G_CONSTANT i32 3
+    ; CHECK: [[SHL:%[0-9]+]]:_(s32) = G_SHL [[COPY]], [[C]](s32)
+    ; CHECK: [[INT:%[0-9]+]]:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.readfirstlane), [[SHL]](s32)
+    ; CHECK: $sgpr0 = COPY [[INT]](s32)
+    ; CHECK: SI_RETURN_TO_EPILOG implicit $sgpr0
+    %0:_(s32) = COPY $vgpr0
+    %10:_(s32) = G_CONSTANT i32 3
+    %5:_(s32) = G_SHL %0, %10(s32)
+    %3:_(p3) = G_CONSTANT i32 0
+    %6:_(p3) = G_PTR_ADD %3, %5(s32)
+    %8:_(s32) = G_PTRTOINT %6(p3)
+    %9:_(s32) = G_INTRINSIC intrinsic(@llvm.amdgcn.readfirstlane), %8(s32)
+    $sgpr0 = COPY %9(s32)
+    SI_RETURN_TO_EPILOG implicit $sgpr0
+
+...