Skip to content

Conversation

preames
Copy link
Collaborator

@preames preames commented Sep 25, 2025

We didn't have trace logging for two cases in this routine which makes it sometimes hard to tell what is going on. In addition to debug trace statements, add comments to explain the logic behind the early exits which don't mark the virtual register live. Suggestions on how to word these more precisely very welcome; I'm not clear I understand all the intrinicies of this code myself.

We didn't have trace logging for two cases in this routine which makes
it sometimes hard to tell what is going on.  In addition to debug trace
statements, add comments to explain the logic behind the early exits
which don't mark the virtual register live.  Suggestions on how to
word these more precisely very welcome; I'm not clear I understand all
the intrinicies of this code myself.
@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2025

@llvm/pr-subscribers-llvm-regalloc

Author: Philip Reames (preames)

Changes

We didn't have trace logging for two cases in this routine which makes it sometimes hard to tell what is going on. In addition to debug trace statements, add comments to explain the logic behind the early exits which don't mark the virtual register live. Suggestions on how to word these more precisely very welcome; I'm not clear I understand all the intrinicies of this code myself.


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

1 Files Affected:

  • (modified) llvm/lib/CodeGen/InlineSpiller.cpp (+11-2)
diff --git a/llvm/lib/CodeGen/InlineSpiller.cpp b/llvm/lib/CodeGen/InlineSpiller.cpp
index 98c56f739ad4e..0c2b74c907d2a 100644
--- a/llvm/lib/CodeGen/InlineSpiller.cpp
+++ b/llvm/lib/CodeGen/InlineSpiller.cpp
@@ -642,8 +642,12 @@ bool InlineSpiller::reMaterializeFor(LiveInterval &VirtReg, MachineInstr &MI) {
   SmallVector<std::pair<MachineInstr *, unsigned>, 8> Ops;
   VirtRegInfo RI = AnalyzeVirtRegInBundle(MI, VirtReg.reg(), &Ops);
 
-  if (!RI.Reads)
+  // Defs without reads will be deleted if unused after remat is
+  // completed for other users of the virtual register.
+  if (!RI.Reads) {
+    LLVM_DEBUG(dbgs() << "\tskipping remat of def " << MI);
     return false;
+  }
 
   SlotIndex UseIdx = LIS.getInstructionIndex(MI).getRegSlot(true);
   VNInfo *ParentVNI = VirtReg.getVNInfoAt(UseIdx.getBaseIndex());
@@ -657,8 +661,13 @@ bool InlineSpiller::reMaterializeFor(LiveInterval &VirtReg, MachineInstr &MI) {
     return true;
   }
 
-  if (SnippetCopies.count(&MI))
+  // Snippets copies are ignored for remat, and will be deleted if they
+  // don't feed a live user after rematerialization completes.
+  if (SnippetCopies.count(&MI)) {
+    LLVM_DEBUG(dbgs() << "\tskipping remat snippet copy for " << UseIdx << '\t'
+                      << MI);
     return false;
+  }
 
   LiveInterval &OrigLI = LIS.getInterval(Original);
   VNInfo *OrigVNI = OrigLI.getVNInfoAt(UseIdx);

@preames preames merged commit 78da592 into llvm:main Sep 26, 2025
12 checks passed
@preames preames deleted the pr-inlinespiller-trace-remat-defs-and-snippets branch September 26, 2025 13:54
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
…lvm#160761)

We didn't have trace logging for two cases in this routine which makes
it sometimes hard to tell what is going on. In addition to debug trace
statements, add comments to explain the logic behind the early exits
which don't mark the virtual register live. Suggestions on how to word
these more precisely very welcome; I'm not clear I understand all the
intrinicies of this code myself.
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