[PATCH] D98232: [regalloc] Ensure Query::collectInterferringVregs is called before interval iteration

Wed Mar 10 11:14:32 PST 2021

qcolombet added inline comments.

================
Comment at: llvm/test/CodeGen/AArch64/ragreedy-local-interval-cost.ll:67
 ; CHECK-NEXT:    ldr q15, [x8]
+; CHECK-NEXT:    mov v28.16b, v12.16b
+; CHECK-NEXT:    mov v12.16b, v10.16b
----------------
dmgreen wrote:
> qcolombet wrote:
> > dmgreen wrote:
> > > This is quite a large regression. My understanding is that for this test enableAdvancedRASplitCost/consider-local-interval-cost was enabled specifically to prevent this kind of recursive spilling from happening.
> > The problem is that the previous version was relying on stall information.
> > This is a correctness fix and should get in ASAP IMHO.
> > 
> > As far as eviction chains go, we could actually clean them up later in the pipeline.
> > @aditya_nandakumar worked on a prototype offline that solves that as a post RA pass. We're hoping to open source it soon-ish.
> If this is a correctness fix, presumably there should be a new test case for what it is fixing?
Good point!

>From what I understand this is a subtle issue because I don't think we can generate wrong code out of it, it will just do not detect eviction chain properly, or have the compiler spins forever (IIUC what @mtrofin means by "the compiler ices").

I let @mtrofin comment on this.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98232/new/

https://reviews.llvm.org/D98232