RegisterCoalescer: Set undef on full register uses when coalescing implicit_def

[arsenm]

Previously this would delete the IMPLICIT_DEF and not introduce the undef
flag on the use operand.

Fixes sub-issue found while reducing #109294

[arsenm]

• RegisterCoalescer: Set undef on full register uses when coalescing implicit_def #118321 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-llvm-regalloc

Author: Matt Arsenault (arsenm)

Changes

Previously this would delete the IMPLICIT_DEF and not introduce the undef
flag on the use operand.

Fixes sub-issue found while reducing #109294

---

Full diff: https://github.com/llvm/llvm-project/pull/118321.diff

2 Files Affected:

• (modified) llvm/lib/CodeGen/RegisterCoalescer.cpp (+4-1)
• (added) llvm/test/CodeGen/AMDGPU/register-coalescer-issue109294.mir (+43)

diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index 5de873fd41578e..20ad6445344d83 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -1842,9 +1842,12 @@ void RegisterCoalescer::updateRegDefsUses(Register SrcReg, Register DstReg,
 
   if (DstInt && DstInt->hasSubRanges() && DstReg != SrcReg) {
     for (MachineOperand &MO : MRI->reg_operands(DstReg)) {
+      if (MO.isUndef())
+        continue;
       unsigned SubReg = MO.getSubReg();
-      if (SubReg == 0 || MO.isUndef())
+      if (SubReg == 0 && MO.isDef())
         continue;
+
       MachineInstr &MI = *MO.getParent();
       if (MI.isDebugInstr())
         continue;
diff --git a/llvm/test/CodeGen/AMDGPU/register-coalescer-issue109294.mir b/llvm/test/CodeGen/AMDGPU/register-coalescer-issue109294.mir
new file mode 100644
index 00000000000000..3f5df7a7b868a0
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/register-coalescer-issue109294.mir
@@ -0,0 +1,43 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx900 -run-pass=register-coalescer -verify-coalescing -o - %s | FileCheck %s
+
+# Make sure that the undef flag is set on %0 after the IMPLICIT_DEF is
+# deleted when coalescing %0 with %1
+
+---
+name:            test
+tracksRegLiveness: true
+machineFunctionInfo:
+  stackPtrOffsetReg: '$sgpr32'
+body:             |
+  bb.0:
+    ; CHECK-LABEL: name: test
+    ; CHECK: [[S_BUFFER_LOAD_DWORDX2_IMM:%[0-9]+]]:sreg_64_xexec = S_BUFFER_LOAD_DWORDX2_IMM undef %0, 36, 0 :: (dereferenceable invariant load (s64))
+    ; CHECK-NEXT: undef [[S_ADD_U32_:%[0-9]+]].sub1:sgpr_128 = S_ADD_U32 [[S_BUFFER_LOAD_DWORDX2_IMM]].sub0, 32, implicit-def dead $scc
+    ; CHECK-NEXT: SI_RETURN implicit [[S_ADD_U32_]].sub1
+    %0:sgpr_128 = IMPLICIT_DEF
+    %1:sreg_64_xexec = S_BUFFER_LOAD_DWORDX2_IMM %0, 36, 0 :: (dereferenceable invariant load (s64))
+    %2:sreg_32 = S_ADD_U32 %1.sub0, 32, implicit-def dead $scc
+    %0.sub1:sgpr_128 = COPY killed %2
+    SI_RETURN implicit %0.sub1
+
+...
+
+---
+name:            test_w_undef_dead
+tracksRegLiveness: true
+machineFunctionInfo:
+  stackPtrOffsetReg: '$sgpr32'
+body:             |
+  bb.0:
+    ; CHECK-LABEL: name: test_w_undef_dead
+    ; CHECK: dead [[S_BUFFER_LOAD_DWORDX2_IMM:%[0-9]+]]:sreg_64_xexec = S_BUFFER_LOAD_DWORDX2_IMM undef %0, 36, 0 :: (dereferenceable invariant load (s64))
+    ; CHECK-NEXT: undef [[S_ADD_U32_:%[0-9]+]].sub1:sgpr_128 = S_ADD_U32 undef [[S_BUFFER_LOAD_DWORDX2_IMM]].sub0, 32, implicit-def dead $scc
+    ; CHECK-NEXT: SI_RETURN implicit [[S_ADD_U32_]].sub1
+    %0:sgpr_128 = IMPLICIT_DEF
+    dead %1:sreg_64_xexec = S_BUFFER_LOAD_DWORDX2_IMM %0, 36, 0 :: (dereferenceable invariant load (s64))
+    %2:sreg_32 = S_ADD_U32 undef %1.sub0, 32, implicit-def dead $scc
+    %0.sub1:sgpr_128 = COPY killed %2
+    SI_RETURN implicit %0.sub1
+
+...

[qcolombet]

LGTM

[arsenm]

Merge activity

• Dec 2, 2:42 PM EST: A user started a stack merge that includes this pull request via Graphite.
• Dec 2, 2:43 PM EST: A user merged this pull request with Graphite.

[llvm-ci]

LLVM Buildbot has detected a new failure on builder openmp-offload-sles-build-only running on rocm-worker-hw-04-sles while building llvm at step 8 "Add check check-llvm".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/12110

Here is the relevant piece of the build log for the reference

Step 8 (Add check check-llvm) failure: 1200 seconds without output running [b'ninja', b'-j 32', b'check-llvm'], attempting to kill
...
[681/685] Linking CXX executable unittests/Transforms/Scalar/ScalarTests
[682/685] Linking CXX executable unittests/Target/TargetMachineCTests
[683/685] Linking CXX executable unittests/tools/llvm-exegesis/LLVMExegesisTests
[684/685] Linking CXX executable unittests/Target/AMDGPU/AMDGPUTests
[684/685] Running the LLVM regression tests
llvm-lit: /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/utils/lit/lit/llvm/config.py:506: note: using ld.lld: /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.build/bin/ld.lld
llvm-lit: /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/utils/lit/lit/llvm/config.py:506: note: using lld-link: /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.build/bin/lld-link
llvm-lit: /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/utils/lit/lit/llvm/config.py:506: note: using ld64.lld: /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.build/bin/ld64.lld
llvm-lit: /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/llvm/utils/lit/lit/llvm/config.py:506: note: using wasm-ld: /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.build/bin/wasm-ld
-- Testing: 56373 tests, 32 workers --
command timed out: 1200 seconds without output running [b'ninja', b'-j 32', b'check-llvm'], attempting to kill
process killed by signal 9
program finished with exit code -1
elapsedTime=1288.887750
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..