Skip to content

Conversation

preames
Copy link
Collaborator

@preames preames commented Sep 26, 2025

Reverts #160765. Failures on buildbot indicate second assertion does not in fact hold.

@preames preames merged commit 9412769 into main Sep 26, 2025
8 of 10 checks passed
@preames preames deleted the revert-160765-pr-inlinespiller-scanRemat-asserts branch September 26, 2025 14:56
@llvmbot
Copy link
Member

llvmbot commented Sep 26, 2025

@llvm/pr-subscribers-llvm-regalloc

Author: Philip Reames (preames)

Changes

Reverts llvm/llvm-project#160765. Failures on buildbot indicate second assertion does not in fact hold.


Full diff: https://github.com/llvm/llvm-project/pull/160897.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 4aeacc332476d..59bc82dc267b5 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);
-    assert(OrigVNI && "Corrupt interval mapping?");
-    if (OrigVNI->isPHIDef())
+    if (!OrigVNI)
       continue;
     MachineInstr *DefMI = LIS.getInstructionFromIndex(OrigVNI->def);
-    assert(DefMI && "Missing instruction for def slot");
+    if (!DefMI)
+      continue;
     if (TII.isReMaterializable(*DefMI))
       Remattable.insert(OrigVNI);
   }

@arsenm
Copy link
Contributor

arsenm commented Sep 26, 2025

Reverts #160765. Failures on buildbot indicate second assertion does not in fact hold.

Include reduced sample?

@preames
Copy link
Collaborator Author

preames commented Sep 26, 2025

Reverts #160765. Failures on buildbot indicate second assertion does not in fact hold.

Include reduced sample?

Don't yet have one. It looks like a stage2 compile failed, but only on the riscv bots so far, which is weird. I do plan to try and extract a test case here, but wanted to get the bots clean first.

@preames
Copy link
Collaborator Author

preames commented Sep 30, 2025

Include reduced sample?

Don't yet have one.

@asb helped me to reduce this down to the following:

$ cat reduced.ll 
target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n32:64-S128"
target triple = "riscv64-unknown-linux-gnu"

define i64 @clause_IsUnorderedClause(i64 %0, <vscale x 16 x i64> %1,
<vscale x 16 x i64> %2) #0 {
entry:
  %broadcast.splatinsert = insertelement <vscale x 16 x i64>
zeroinitializer, i64 %0, i64 0
  %broadcast.splat = shufflevector <vscale x 16 x i64>
%broadcast.splatinsert, <vscale x 16 x i64> zeroinitializer, <vscale x
16 x i32> zeroinitializer
  br label %vector.body

vector.body:                                      ; preds =
%vector.body, %entry
  %vec.ind = phi <vscale x 16 x i64> [ zeroinitializer, %entry ], [
%vec.ind.next, %vector.body ]
  %3 = and <vscale x 16 x i64> %vec.ind, %broadcast.splat
  %4 = icmp ne <vscale x 16 x i64> %3, zeroinitializer
  %5 = tail call i1 @llvm.vector.reduce.or.nxv16i1(<vscale x 16 x i1>
%4)
  %vec.ind.next = or <vscale x 16 x i64> %vec.ind, %1
  br i1 %5, label %middle.block, label %vector.body

middle.block:                                     ; preds = %vector.body
  %and.i = and i64 1, %0
  ret i64 %and.i

; uselistorder directives
  uselistorder i64 %0, { 1, 0 }
}

; Function Attrs: nocallback nofree nosync nounwind speculatable
willreturn memory(none)
declare i1 @llvm.vector.reduce.or.nxv16i1(<vscale x 16 x i1>) #1

attributes #0 = { "target-features"="+v" }
attributes #1 = { nocallback nofree nosync nounwind speculatable
willreturn memory(none) }

What appears to be going on is that the original definition has been removed from the parent interval in a previous round of rematerialization. We had materialized to all uses once, and then end up coming around and trying to rematerialize the remat introduced def a second time. (This is not crazy, we've further split a live interval in the intermediate time.)

This is in turn due to an interaction in LRE::eliminateDeadDefs that I don't understand and need to stare at a bit more.

@preames
Copy link
Collaborator Author

preames commented Oct 2, 2025

Adding test coverage for the issue leading to the revert: #161614

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