Skip to content

Conversation

preames
Copy link
Collaborator

@preames preames commented Sep 25, 2025

We should always be able to find the VNInfo in the original live interval which corresponds to the subset we're trying to spill, and the only cases where we have a VNInfo without a definition instruction are if the vni is unused, or corresponds to a phi. Adjust the code structure to explicitly check for PHIDef, and assert the stronger conditions.

We should always be able to find the VNInfo in the original live
interval which corresponds to the subset we're trying to spill,
and the only cases where we have a VNInfo without a definition
instruction are if the vni is unused, or corresponds to a phi.
Adjust the code structure to explicitly check for PHIDef, and
assert the stronger conditions.
@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2025

@llvm/pr-subscribers-llvm-regalloc

Author: Philip Reames (preames)

Changes

We should always be able to find the VNInfo in the original live interval which corresponds to the subset we're trying to spill, and the only cases where we have a VNInfo without a definition instruction are if the vni is unused, or corresponds to a phi. Adjust the code structure to explicitly check for PHIDef, and assert the stronger conditions.


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

1 Files Affected:

  • (modified) llvm/lib/CodeGen/LiveRangeEdit.cpp (+3-3)
diff --git a/llvm/lib/CodeGen/LiveRangeEdit.cpp b/llvm/lib/CodeGen/LiveRangeEdit.cpp
index 2f65be51bb726..dc1121d7d9e28 100644
--- a/llvm/lib/CodeGen/LiveRangeEdit.cpp
+++ b/llvm/lib/CodeGen/LiveRangeEdit.cpp
@@ -75,11 +75,11 @@ void LiveRangeEdit::scanRemattable() {
     Register Original = VRM->getOriginal(getReg());
     LiveInterval &OrigLI = LIS.getInterval(Original);
     VNInfo *OrigVNI = OrigLI.getVNInfoAt(VNI->def);
-    if (!OrigVNI)
+    assert(OrigVNI && "Corrupt interval mapping?");
+    if (OrigVNI->isPHIDef())
       continue;
     MachineInstr *DefMI = LIS.getInstructionFromIndex(OrigVNI->def);
-    if (!DefMI)
-      continue;
+    assert(DefMI && "Missing instruction for def slot");
     if (TII.isReMaterializable(*DefMI))
       Remattable.insert(OrigVNI);
   }

@preames
Copy link
Collaborator Author

preames commented Sep 25, 2025

Slightly skeptical review is appreciated. I want to make sure I'm not missing something in the invariants here.

@preames preames merged commit bba9172 into llvm:main Sep 26, 2025
12 checks passed
@preames preames deleted the pr-inlinespiller-scanRemat-asserts branch September 26, 2025 13:55
preames added a commit that referenced this pull request Sep 26, 2025
…e [nfc]" (#160897)

Reverts #160765. Failures on buildbot indicate second
assertion does not in fact hold.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Sep 26, 2025
…anRemattable [nfc]" (#160897)

Reverts llvm/llvm-project#160765. Failures on buildbot indicate second
assertion does not in fact hold.
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
…lvm#160765)

We should always be able to find the VNInfo in the original live
interval which corresponds to the subset we're trying to spill, and the
only cases where we have a VNInfo without a definition instruction are
if the vni is unused, or corresponds to a phi. Adjust the code structure
to explicitly check for PHIDef, and assert the stronger conditions.
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
…e [nfc]" (llvm#160897)

Reverts llvm#160765. Failures on buildbot indicate second
assertion does not in fact hold.
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