From 524946d2475d0b541ba84396f759b9e4c7aa4a98 Mon Sep 17 00:00:00 2001 From: "khr@golang.org" Date: Tue, 13 May 2025 17:53:45 -0700 Subject: [PATCH] cmd/compile: don't preload registers if destination already scheduled In regalloc, we allocate some values to registers before loop entry, so that they don't need to be loaded (from spill locations) during the loop. But it is pointless if we've already regalloc'd the loop body. Whatever restores we needed for the body are already generated. It's not clear if this code is ever useful. No tests fail if I just remove it. But at least this change is worthwhile. It doesn't help, and it actively inserts more restores than we really need (mostly because the desired register list is approximate - I have seen cases where the loads implicated here end up being dead because the restores hit the wrong registers and the edge shuffle pass knows it needs the restores in different registers). While we are here, might as well have layoutRegallocOrder return the standard layout order instead of recomputing it. Change-Id: Ia624d5121de59b6123492603695de50b272b277f Reviewed-on: https://go-review.googlesource.com/c/go/+/672735 LUCI-TryBot-Result: Go LUCI Reviewed-by: Keith Randall Reviewed-by: David Chase --- src/cmd/compile/internal/ssa/layout.go | 2 +- src/cmd/compile/internal/ssa/regalloc.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/cmd/compile/internal/ssa/layout.go b/src/cmd/compile/internal/ssa/layout.go index e4a8c6ffbf..927287dc77 100644 --- a/src/cmd/compile/internal/ssa/layout.go +++ b/src/cmd/compile/internal/ssa/layout.go @@ -15,7 +15,7 @@ func layout(f *Func) { // imposed by the linear-scan algorithm. func layoutRegallocOrder(f *Func) []*Block { // remnant of an experiment; perhaps there will be another. - return layoutOrder(f) + return f.Blocks } func layoutOrder(f *Func) []*Block { diff --git a/src/cmd/compile/internal/ssa/regalloc.go b/src/cmd/compile/internal/ssa/regalloc.go index 2981bceb2e..f1e210fe9b 100644 --- a/src/cmd/compile/internal/ssa/regalloc.go +++ b/src/cmd/compile/internal/ssa/regalloc.go @@ -1899,6 +1899,10 @@ func (s *regAllocState) regalloc(f *Func) { if s.f.Config.hasGReg && s.regs[s.GReg].v != nil { s.freeReg(s.GReg) // Spill value in G register before any merge. } + if s.blockOrder[b.ID] > s.blockOrder[b.Succs[0].b.ID] { + // No point if we've already regalloc'd the destination. + goto badloop + } // For this to be worthwhile, the loop must have no calls in it. top := b.Succs[0].b loop := s.loopnest.b2l[top.ID] -- 2.51.0