Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RegisterCoalescer: Relax assert for super register def rematerialization #69088

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Oct 15, 2023

Depending on the uses, the super register def added when coalescing SUBREG_TO_REG may have been reduced to some set of used subregisters so we may not just see a simple super register reference.

@llvmbot
Copy link

llvmbot commented Oct 15, 2023

@llvm/pr-subscribers-llvm-regalloc

@llvm/pr-subscribers-backend-x86

Author: Matt Arsenault (arsenm)

Changes

Depending on the uses, the super register def added when coalescing SUBREG_TO_REG may have been reduced to some set of used subregisters so we may not just see a simple super register reference.


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

3 Files Affected:

  • (modified) llvm/lib/CodeGen/RegisterCoalescer.cpp (+10-4)
  • (added) llvm/test/CodeGen/X86/coalescer-implicit-def-regression-imp-operand-assert.mir (+39)
  • (added) llvm/test/CodeGen/X86/coalescer-implicit-def-regression.mir (+91)
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index 7e5ce300370c92b..78c5ccde5d626a6 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -1415,6 +1415,9 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
   // from SUBREG_TO_REG, such as:
   // $edi = MOV32r0 implicit-def dead $eflags, implicit-def $rdi
   // undef %0.sub_32bit = MOV32r0 implicit-def dead $eflags, implicit-def %0
+  //
+  // The implicit-def of the super register may have been reduced to
+  // subregisters depending on the uses.
 
   bool NewMIDefinesFullReg = false;
 
@@ -1432,12 +1435,15 @@ bool RegisterCoalescer::reMaterializeTrivialDef(const CoalescerPair &CP,
         assert(MO.isImplicit() && MO.getReg().isPhysical() &&
                (MO.isDead() ||
                 (DefSubIdx &&
-                 (TRI->getSubReg(MO.getReg(), DefSubIdx) ==
-                  MCRegister((unsigned)NewMI.getOperand(0).getReg())))));
+                 ((TRI->getSubReg(MO.getReg(), DefSubIdx) ==
+                   MCRegister((unsigned)NewMI.getOperand(0).getReg())) ||
+                  TRI->isSubRegisterEq(NewMI.getOperand(0).getReg(), MO.getReg())
+                   )))
+                 );
         NewMIImplDefs.push_back(MO.getReg().asMCReg());
       } else {
-        assert(MO.getReg() == NewMI.getOperand(0).getReg() &&
-               MO.getSubReg() == 0);
+        assert(MO.getReg() == NewMI.getOperand(0).getReg());
+
         // We're only expecting another def of the main output, so the range
         // should get updated with the regular output range.
         //
diff --git a/llvm/test/CodeGen/X86/coalescer-implicit-def-regression-imp-operand-assert.mir b/llvm/test/CodeGen/X86/coalescer-implicit-def-regression-imp-operand-assert.mir
new file mode 100644
index 000000000000000..14220ee01131f9a
--- /dev/null
+++ b/llvm/test/CodeGen/X86/coalescer-implicit-def-regression-imp-operand-assert.mir
@@ -0,0 +1,39 @@
+# RUN: llc -mtriple=x86_64-unknown-linux-gnu -run-pass=register-coalescer -o - %s
+---
+name:  rematerialize_subreg_to_reg_added_impdef_1
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    successors: %bb.1(0x2aaaaaab), %bb.2(0x55555555)
+    liveins: $edi
+
+    %0:gr32 = MOV32r0 implicit-def dead $eflags
+    %1:gr8 = COPY %0.sub_8bit
+    %2:gr64 = SUBREG_TO_REG 0, killed %0, %subreg.sub_32bit
+    JCC_1 %bb.2, 5, implicit killed undef $eflags
+
+  bb.1:
+    successors: %bb.3(0x80000000)
+
+    JMP_1 %bb.3
+
+  bb.2:
+    successors: %bb.3(0x80000000)
+
+    %5:gr64 = IMPLICIT_DEF
+    %2:gr64 = COPY killed %5
+
+  bb.3:
+    successors: %bb.4(0x30000000), %bb.5(0x50000000)
+
+    JCC_1 %bb.5, 5, implicit killed undef $eflags
+
+  bb.4:
+    $al = COPY killed %1
+    RET 0, killed undef $al
+
+  bb.5:
+    MOV64mr undef $noreg, 1, undef $noreg, 0, undef $noreg, killed %2 :: (store (s64))
+    RET 0, killed undef $al
+
+...
diff --git a/llvm/test/CodeGen/X86/coalescer-implicit-def-regression.mir b/llvm/test/CodeGen/X86/coalescer-implicit-def-regression.mir
new file mode 100644
index 000000000000000..722d9b60841a278
--- /dev/null
+++ b/llvm/test/CodeGen/X86/coalescer-implicit-def-regression.mir
@@ -0,0 +1,91 @@
+# RUN: llc -mtriple=x86_64-unknown-linux-gnu -run-pass=register-coalescer -o - %s
+
+# An implicit-def will be added to SUBREG_TO_REG during coalescing
+---
+name:  rematerialize_subreg_to_reg_added_impdef_0
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    successors: %bb.1(0x2aaaaaab), %bb.2(0x55555555)
+    liveins: $edi
+
+    %0:gr32 = MOV32r0 implicit-def dead $eflags
+    %1:gr8 = COPY %0.sub_8bit
+    %2:gr64 = SUBREG_TO_REG 0, killed %0, %subreg.sub_32bit
+    JCC_1 %bb.2, 5, implicit killed undef $eflags
+
+  bb.1:
+    %4:gr8 = COPY %1
+    %5:gr8 = COPY killed undef %1
+    JMP_1 %bb.5
+
+  bb.2:
+    %6:gr64 = IMPLICIT_DEF
+    %2:gr64 = COPY killed %6
+    %5:gr8 = MOV8ri 1
+
+  bb.5:
+    successors: %bb.6(0x30000000), %bb.7(0x50000000)
+
+    TEST8rr killed undef %5, %5, implicit-def $eflags
+    JCC_1 %bb.7, 5, implicit killed undef $eflags
+
+  bb.6:
+    $al = COPY killed %1
+    RET 0, killed undef $al
+
+  bb.7:
+    MOV64mr undef $noreg, 1, undef $noreg, 0, undef $noreg, killed %2 :: (store (s64))
+    RET 0, killed undef $al
+
+...
+
+
+# Reduced version of previous with the SUBREG_TO_REG already folded
+# away.
+#
+# The mov32r0 defines a subregister and has an implicit-def of the
+# super register. After coalescing, the full register implicit def of
+# %2 becomes a different subregister def.
+
+---
+name:  rematerialize_subreg_to_reg_coalesces_to_subreg_impdef
+tracksRegLiveness: true
+body:             |
+  bb.0:
+    successors: %bb.1(0x2aaaaaab), %bb.2(0x55555555)
+    liveins: $edi
+
+    undef %2.sub_32bit:gr64_with_sub_8bit = MOV32r0 implicit-def dead $eflags, implicit-def %2
+    %1:gr8 = COPY %2.sub_8bit
+    JCC_1 %bb.2, 5, implicit killed undef $eflags
+
+  bb.1:
+    successors: %bb.3(0x80000000)
+
+    dead %3:gr8 = COPY %1
+    %4:gr8 = COPY undef %1
+    JMP_1 %bb.3
+
+  bb.2:
+    successors: %bb.3(0x80000000)
+
+    %5:gr64 = IMPLICIT_DEF
+    %2:gr64_with_sub_8bit = COPY %5
+    %4:gr8 = MOV8ri 1
+
+  bb.3:
+    successors: %bb.4(0x30000000), %bb.5(0x50000000)
+
+    TEST8rr undef %4, %4, implicit-def $eflags
+    JCC_1 %bb.5, 5, implicit killed undef $eflags
+
+  bb.4:
+    $al = COPY %1
+    RET 0, killed undef $al
+
+  bb.5:
+    MOV64mr undef $noreg, 1, undef $noreg, 0, undef $noreg, %2 :: (store (s64))
+    RET 0, killed undef $al
+
+...

@github-actions
Copy link

github-actions bot commented Oct 15, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

Depending on the uses, the super register def added when coalescing
SUBREG_TO_REG may have been reduced to some set of used subregisters so we
may not just see a simple super register reference.
@arsenm arsenm force-pushed the coalescer-relax-impdef-assert branch from 19c142e to 3ada9a0 Compare October 15, 2023 23:58
@arsenm arsenm merged commit e62d25e into llvm:main Oct 31, 2023
3 checks passed
Guzhu-AMD pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Nov 2, 2023
Local branch amd-gfx 2baaf7c Merged main:a6dabed3483c into amd-gfx:adee0826382f
Remote branch main e62d25e RegisterCoalescer: Relax assert for super register def rematerialization (llvm#69088)
@arsenm arsenm deleted the coalescer-relax-impdef-assert branch April 3, 2024 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants