[GlobalISel] Correct constant type in matchReassocConstantInnerLHS

[bcl5980]

Summary

When we match a pattern from m_GCst, the register type could be different from original op. So we can't replace the original op to vreg direct.
This code create a new constant with original op type then replace the original op.

Fix #58906

[arsenm]

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

4552–4561

If you're going to potentially create a new constant anyway, can you just create the constant and get rid of all this moveBefore stuff?

[arsenm]

llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-ptradd-chain.mir

16

Should precommit the tests changes that change all the CHECK to CHECK-NEXT so the diff is readable

[arsenm]

llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-ptradd-chain.mir

16

I've done this for you in 9fe5ca9b307614518f2d075587260eff16e462de

[bcl5980]

Address comments.

[arsenm]

llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-ptradd-chain.mir

109

Can you add a reference to the issue number here

[bcl5980]

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

4552–4561

It looks we can't remove the moveBefore. Test failed on CodeGen/AMDGPU/GlobalISel/mubuf-global.ll.

[arsenm]

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

4552–4561

By failed do you mean actually broken, or just needs updating?

[bcl5980]

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

4552–4561

Actually broken.

Use of %10 does not have a corresponding definition on every path:
64r %31:vgpr_32, %33:sreg_64_xexec = V_ADD_CO_U32_e64 %3:vgpr_32, %10.sub0:vreg_64, 0, implicit $exec
LLVM ERROR: Use not jointly dominated by defs.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0. Program arguments: build\\bin\\llc -global-isel -mtriple=amdgcn-mesa-mesa3d -mcpu=tahiti

1. Running pass 'CallGraph Pass Manager' on module '<stdin>'.
2. Running pass 'Live Interval Analysis' on function '@mubuf_store_vgpr_ptr_sgpr_offset256_offset'

[bcl5980]

@arsenm How about this patch now ?

[aemerson]

LGTM, thanks!

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

if (!mi_match(MI.getBaseReg(), MRI,
                 m_OneNonDBGUse(m_GPtrAdd(m_Reg(LHSBase), m_GCst(LHSCstOff)))))
     return false;
 
   auto *LHSPtrAdd = cast<GPtrAdd>(LHS);
   MatchInfo = [=, &MI](MachineIRBuilder &B) {
     // When we change LHSPtrAdd's offset register we might cause it to use a reg
     // before its def. Sink the instruction so the outer PTR_ADD to ensure this
     // doesn't happen.
     LHSPtrAdd->moveBefore(&MI);
     Register RHSReg = MI.getOffsetReg();
+    // set VReg will cause type mismatch if it comes from extend/trunc
+    auto NewCst = B.buildConstant(MRI.getType(RHSReg), LHSCstOff->Value);
     Observer.changingInstr(MI);
-    MI.getOperand(2).setReg(LHSCstOff->VReg);
+    MI.getOperand(2).setReg(NewCst.getReg(0));
     Observer.changedInstr(MI);
     Observer.changingInstr(*LHSPtrAdd);
     LHSPtrAdd->getOperand(2).setReg(RHSReg);
     Observer.changedInstr(*LHSPtrAdd);
   };
   return !reassociationCanBreakAddressingModePattern(MI);
 }
 
 bool CombinerHelper::matchReassocFoldConstantsInSubTree(GPtrAdd &MI,
                                                         MachineInstr *LHS,
                                                         MachineInstr *RHS,
                                                         BuildFnTy &MatchInfo) {

llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-ptradd-chain.mir

 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
-# RUN: llc -mtriple aarch64-apple-ios  -run-pass=aarch64-prelegalizer-combiner --aarch64prelegalizercombinerhelper-only-enable-rule="ptr_add_immed_chain"  %s -o - -verify-machineinstrs | FileCheck %s
+# RUN: llc -mtriple aarch64-apple-ios  -run-pass=aarch64-prelegalizer-combiner  %s -o - -verify-machineinstrs | FileCheck %s
 # REQUIRES: asserts
 
 # Check that we fold two adds of constant offsets with G_PTR_ADD into a single G_PTR_ADD.
 ---
 name: ptradd_chain
 tracksRegLiveness: true
 body:             |
   bb.1:
     liveins: $x0
 
     ; CHECK-LABEL: name: ptradd_chain
     ; CHECK: liveins: $x0
     ; CHECK-NEXT: {{  $}}
     ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
     ; CHECK-NEXT: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 16
     ; CHECK-NEXT: [[PTR_ADD:%[0-9]+]]:_(p0) = G_PTR_ADD [[COPY]], [[C]](s64)
     ; CHECK-NEXT: $x0 = COPY [[PTR_ADD]](p0)
     ; CHECK-NEXT: RET_ReallyLR implicit $x0
     %0:_(p0) = COPY $x0
     %1:_(s64) = G_CONSTANT i64 4
     %2:_(s64) = G_CONSTANT i64 12
     %3:_(p0) = G_PTR_ADD %0(p0), %1
bb.1:
     %3:_(p0) = G_PTR_ADD %0(p0), %1
     %4:_(p0) = G_PTR_ADD %3(p0), %2
     %ld:_(s64) = G_LOAD %4(p0) :: (load 8)
     %ld_other:_(s64) = G_LOAD %3(p0) :: (load 8)
     $x0 = COPY %ld(s64)
     $x1 = COPY %ld_other(s64)
     RET_ReallyLR implicit $x0
 ...
+---
+# PR58906
+name: ptradd_constant_type_mismatch
+tracksRegLiveness: true
+body:             |
+  bb.1:
+    liveins: $x0, $x1
+
+    ; CHECK-LABEL: name: ptradd_constant_type_mismatch
+    ; CHECK: liveins: $x0, $x1
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s64) = COPY $x1
+    ; CHECK-NEXT: [[PTR_ADD:%[0-9]+]]:_(p0) = G_PTR_ADD [[COPY]], [[COPY1]](s64)
+    ; CHECK-NEXT: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 8
+    ; CHECK-NEXT: [[PTR_ADD1:%[0-9]+]]:_(p0) = G_PTR_ADD [[PTR_ADD]], [[C]](s64)
+    ; CHECK-NEXT: %ld:_(s64) = G_LOAD [[PTR_ADD1]](p0) :: (load (s64))
+    ; CHECK-NEXT: $x0 = COPY %ld(s64)
+    ; CHECK-NEXT: RET_ReallyLR implicit $x0
+    %0:_(p0) = COPY $x0
+    %1:_(s64) = COPY $x1
+    %2:_(s32) = G_CONSTANT i32 8
+    %3:_(s64) = G_ZEXT %2:_(s32)
+    %4:_(p0) = G_PTR_ADD %0:_, %3:_(s64)
+    %5:_(p0) = G_PTR_ADD %4:_, %1:_(s64)
+    %ld:_(s64) = G_LOAD %5(p0) :: (load 8)
+    $x0 = COPY %ld(s64)
+    RET_ReallyLR implicit $x0
+...