[Statepoint] Treat undef operands less specially

[arsenm]

This reverts commit f744390.

This is to avoid an assertion if an undef operand appears in a
stackmap. This is important to avoid hitting verifier errors
when register allocation starts adding undefs in error scenarios.

Rather than trying to treat undef operands as special, leave them
alone and avoid producing an invalid spill. It would a bit more
precise to produce a spill of an undef register here, but that's not
exposed through the storeRegToStackSlot API.

https://reviews.llvm.org/D122605

This was an alternative to https://reviews.llvm.org/D122582

[arsenm]

• RegAlloc: Fix verifier error after failed allocation #119690
• AMDGPU: Delete spills of undef values #119684
• [Statepoint] Treat undef operands less specially #119682 👈 (View in Graphite)
• RegAlloc: Fix failure on undef use when all registers are reserved #119647
• RegAlloc: Do not fatal error if there are no registers in the alloc order #119640
• RegAlloc: Use DiagnosticInfo to report register allocation failures #119492
• DiagnosticPrinter: Use printAsOperand to handle anonymous values #119491
• DiagnosticInfo: Clean up usage of DiagnosticInfoInlineAsm #119485
• main

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

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: Matt Arsenault (arsenm)

Changes

This reverts commit f744390.

This is to avoid an assertion if an undef operand appears in a
stackmap. This is important to avoid hitting verifier errors
when register allocation starts adding undefs in error scenarios.

Rather than trying to treat undef operands as special, leave them
alone and avoid producing an invalid spill. It would a bit more
precise to produce a spill of an undef register here, but that's not
exposed through the storeRegToStackSlot API.

https://reviews.llvm.org/D122605

This was an alternative to https://reviews.llvm.org/D122582

---

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

4 Files Affected:

• (modified) llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp (-2)
• (modified) llvm/lib/CodeGen/StackMaps.cpp (-6)
• (added) llvm/test/CodeGen/X86/stackmap-undef-operand-anyregcc.mir (+61)
• (modified) llvm/test/CodeGen/X86/statepoint-fixup-undef.mir (+4-4)

diff --git a/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp b/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
index 3bb9da5f1a37bb..0ebe845e473fd6 100644
--- a/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
+++ b/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp
@@ -381,8 +381,6 @@ class StatepointState {
                   EndIdx = MI.getNumOperands();
          Idx < EndIdx; ++Idx) {
       MachineOperand &MO = MI.getOperand(Idx);
-      // Leave `undef` operands as is, StackMaps will rewrite them
-      // into a constant.
       if (!MO.isReg() || MO.isImplicit() || MO.isUndef())
         continue;
       Register Reg = MO.getReg();
diff --git a/llvm/lib/CodeGen/StackMaps.cpp b/llvm/lib/CodeGen/StackMaps.cpp
index 81b288df3b07e0..7480963c1f5217 100644
--- a/llvm/lib/CodeGen/StackMaps.cpp
+++ b/llvm/lib/CodeGen/StackMaps.cpp
@@ -268,12 +268,6 @@ StackMaps::parseOperand(MachineInstr::const_mop_iterator MOI,
     if (MOI->isImplicit())
       return ++MOI;
 
-    if (MOI->isUndef()) {
-      // Record `undef` register as constant. Use same value as ISel uses.
-      Locs.emplace_back(Location::Constant, sizeof(int64_t), 0, 0xFEFEFEFE);
-      return ++MOI;
-    }
-
     assert(MOI->getReg().isPhysical() &&
            "Virtreg operands should have been rewritten before now.");
     const TargetRegisterClass *RC = TRI->getMinimalPhysRegClass(MOI->getReg());
diff --git a/llvm/test/CodeGen/X86/stackmap-undef-operand-anyregcc.mir b/llvm/test/CodeGen/X86/stackmap-undef-operand-anyregcc.mir
new file mode 100644
index 00000000000000..6a322eb105e6f7
--- /dev/null
+++ b/llvm/test/CodeGen/X86/stackmap-undef-operand-anyregcc.mir
@@ -0,0 +1,61 @@
+# RUN: llc -mtriple=x86_64-apple-darwin -start-after=virtregrewriter -o - %s | FileCheck %s
+
+# Check there's no assertion for anyregcc with an undef operand to a stackmap.
+
+# CHECK: __LLVM_StackMaps:
+# CHECK-NEXT: .byte	3
+# CHECK-NEXT: .byte	0
+# CHECK-NEXT: .short	0
+# CHECK-NEXT: .long	1
+# CHECK-NEXT: .long	0
+# CHECK-NEXT: .long	1
+# CHECK-NEXT: .quad	_undef_anyregcc_patchpoint
+# CHECK-NEXT: .quad	8
+# CHECK-NEXT: .quad	1
+# CHECK-NEXT: .quad	12
+# CHECK-NEXT: .long	Ltmp0-_undef_anyregcc_patchpoint
+# CHECK-NEXT: .short	0
+# CHECK-NEXT: .short	2
+# CHECK-NEXT: .byte	1
+# CHECK-NEXT: .byte	0
+# CHECK-NEXT: .short	8
+# CHECK-NEXT: .short	0
+# CHECK-NEXT: .short	0
+# CHECK-NEXT: .long	0
+# CHECK-NEXT: .byte	1
+# CHECK-NEXT: .byte	0
+# CHECK-NEXT: .short	8
+# CHECK-NEXT: .short	0
+# CHECK-NEXT: .short	0
+# CHECK-NEXT: .long	0
+# CHECK-NEXT: .p2align	3
+# CHECK-NEXT: .short	0
+# CHECK-NEXT: .short	2
+# CHECK-NEXT: .short	0
+# CHECK-NEXT: .byte	0
+# CHECK-NEXT: .byte	8
+# CHECK-NEXT: .short	7
+# CHECK-NEXT: .byte	0
+# CHECK-NEXT: .byte	8
+# CHECK-NEXT: .p2align	3
+---
+name:  undef_anyregcc_patchpoint
+tracksRegLiveness: true
+frameInfo:
+  hasPatchPoint:   true
+  hasCalls:        true
+fixedStack:
+  - { id: 0, type: default, offset: 72, size: 8, alignment: 8, stack-id: default,
+      isImmutable: true, isAliased: false, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '', debug-info-expression: '', debug-info-location: '' }
+body:             |
+  bb.0:
+    liveins: $rcx, $rdi, $rdx, $rsi, $r8, $r9
+
+    ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    dead renamable $rax = MOV64rm %fixed-stack.0, 1, $noreg, 0, $noreg :: (load (s64) from %fixed-stack.0)
+    renamable $rax = PATCHPOINT 12, 15, 0, 1, 13, undef renamable $rax, csr_64_allregs, implicit-def dead early-clobber $r11, implicit-def $rsp, implicit-def $ssp
+    ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
+    RET 0, $rax
+
+...
diff --git a/llvm/test/CodeGen/X86/statepoint-fixup-undef.mir b/llvm/test/CodeGen/X86/statepoint-fixup-undef.mir
index 4a18351bde493d..edb0d517d5d52f 100644
--- a/llvm/test/CodeGen/X86/statepoint-fixup-undef.mir
+++ b/llvm/test/CodeGen/X86/statepoint-fixup-undef.mir
@@ -181,13 +181,13 @@ body:             |
   ; STACKMAP-NEXT:	.short	3
   ; STACKMAP-NEXT:	.short	0
   ; STACKMAP-NEXT:	.long	0
-  ;      This is a constant 0xFEFEFEFE we are looking for
-  ; STACKMAP-NEXT:	.byte	4
+  ;      This is entry we're looking for
+  ; STACKMAP-NEXT:	.byte	1
   ; STACKMAP-NEXT:	.byte	0
-  ; STACKMAP-NEXT:	.short	8
+  ; STACKMAP-NEXT:	.short	4
   ; STACKMAP-NEXT:	.short	0
   ; STACKMAP-NEXT:	.short	0
-  ; STACKMAP-NEXT:	.long	-16843010
+  ; STACKMAP-NEXT:	.long	0
   ; STACKMAP-NEXT:	.byte	4
   ; STACKMAP-NEXT:	.byte	0
   ; STACKMAP-NEXT:	.short	8

[arsenm]

Merge activity

• Dec 17, 12:51 AM EST: A user started a stack merge that includes this pull request via Graphite.
• Dec 17, 12:55 AM EST: Graphite rebased this pull request as part of a merge.
• Dec 17, 12:59 AM EST: A user merged this pull request with Graphite.