From 41981930354fdc7bbb59950ce8f303939396b211 Mon Sep 17 00:00:00 2001 From: Matthew Dempsky Date: Fri, 4 Aug 2023 12:53:11 -0700 Subject: [PATCH] cmd/compile/internal/ssagen: sort locals by alignment, not size Sorting variables by size doesn't actually do anything, but sorting by alignment makes sure we can optimally pack variables into the stack frame. While here, replace the monolithic description at the top of cmpstackvarlt with line-by-line explanations of what each comparison is doing. Change-Id: I860677799618130ce4a55f084cec637cb9a2e295 Reviewed-on: https://go-review.googlesource.com/c/go/+/516197 Run-TryBot: Matthew Dempsky Auto-Submit: Matthew Dempsky Reviewed-by: Cuong Manh Le TryBot-Result: Gopher Robot Reviewed-by: Michael Knyszek --- src/cmd/compile/internal/ssagen/pgen.go | 27 +++++++++++++++---------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/cmd/compile/internal/ssagen/pgen.go b/src/cmd/compile/internal/ssagen/pgen.go index 9fd3f2aee4..fd7b2d9cf5 100644 --- a/src/cmd/compile/internal/ssagen/pgen.go +++ b/src/cmd/compile/internal/ssagen/pgen.go @@ -22,44 +22,49 @@ import ( ) // cmpstackvarlt reports whether the stack variable a sorts before b. -// -// Sort the list of stack variables. Autos after anything else, -// within autos, unused after used, within used, things with -// pointers first, zeroed things first, and then decreasing size. -// Because autos are laid out in decreasing addresses -// on the stack, pointers first, zeroed things first and decreasing size -// really means, in memory, things with pointers needing zeroing at -// the top of the stack and increasing in size. -// Non-autos sort on offset. func cmpstackvarlt(a, b *ir.Name) bool { + // Sort non-autos before autos. if needAlloc(a) != needAlloc(b) { return needAlloc(b) } + // If both are non-auto (e.g., parameters, results), then sort by + // frame offset (defined by ABI). if !needAlloc(a) { return a.FrameOffset() < b.FrameOffset() } + // From here on, a and b are both autos (i.e., local variables). + + // Sort used before unused (so AllocFrame can truncate unused + // variables). if a.Used() != b.Used() { return a.Used() } + // Sort pointer-typed before non-pointer types. + // Keeps the stack's GC bitmap compact. ap := a.Type().HasPointers() bp := b.Type().HasPointers() if ap != bp { return ap } + // Group variables that need zeroing, so we can efficiently zero + // them altogether. ap = a.Needzero() bp = b.Needzero() if ap != bp { return ap } - if a.Type().Size() != b.Type().Size() { - return a.Type().Size() > b.Type().Size() + // Sort variables in descending alignment order, so we can optimally + // pack variables into the frame. + if a.Type().Alignment() != b.Type().Alignment() { + return a.Type().Alignment() > b.Type().Alignment() } + // Tie breaker for stable results. return a.Sym().Name < b.Sym().Name } -- 2.50.0