Skip to content

Commit 8566a2d

Browse files
committed
avoid go list for Plan9 assembly discovery
1 parent 8549fc4 commit 8566a2d

3 files changed

Lines changed: 99 additions & 43 deletions

File tree

internal/build/build.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -617,8 +617,9 @@ type context struct {
617617
envInputsCached envSection
618618
envInputsReady bool
619619

620-
// go list derived file lists (SFiles, etc.)
620+
// Plan9 assembly file discovery caches.
621621
sfilesCache map[string][]string // pkg.ID -> absolute .s/.S file paths
622+
asmDirCache map[string]bool // package directory -> whether it contains any .s/.S files
622623

623624
// plan9asm package policy parsed from env.
624625
plan9asmOnce sync.Once

internal/build/plan9asm.go

Lines changed: 45 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,9 @@ package build
22

33
import (
44
"bytes"
5-
"encoding/json"
65
"fmt"
6+
"go/build"
77
"os"
8-
"os/exec"
98
"path/filepath"
109
"strings"
1110
"sync"
@@ -17,12 +16,9 @@ import (
1716
gllvm "github.com/goplus/llvm"
1817
)
1918

20-
// compilePkgSFiles translates Go/Plan9 assembly files selected by `go list -json`
21-
// for this package/target into LLVM IR, compiles them to .o, and returns the
22-
// object files for linking.
23-
//
24-
// NOTE: golang.org/x/tools/go/packages.Package does not expose SFiles, so we
25-
// query `go list -json` here to get the exact filtered set for GOOS/GOARCH.
19+
// compilePkgSFiles translates Go/Plan9 assembly files selected for this
20+
// package/target into LLVM IR, compiles them to .o, and returns the object files
21+
// for linking.
2622
func compilePkgSFiles(ctx *context, aPkg *aPackage, pkg *packages.Package, dynimports []cgoImportDynamicDecl, verbose bool) ([]string, error) {
2723
sfiles, err := pkgSFiles(ctx, pkg)
2824
if err != nil {
@@ -365,6 +361,21 @@ func plan9asmEnabledByDefault(conf *Config, pkgPath string) bool {
365361
return !llruntime.HasAltPkg(pkgPath) || llruntime.HasAdditiveAltPkg(pkgPath)
366362
}
367363

364+
func (c *context) dirHasAsmFile(dir string) bool {
365+
if c == nil {
366+
return dirHasAsmFile(dir)
367+
}
368+
if c.asmDirCache == nil {
369+
c.asmDirCache = make(map[string]bool)
370+
}
371+
if has, ok := c.asmDirCache[dir]; ok {
372+
return has
373+
}
374+
has := dirHasAsmFile(dir)
375+
c.asmDirCache[dir] = has
376+
return has
377+
}
378+
368379
func dirHasAsmFile(dir string) bool {
369380
f, err := os.Open(dir)
370381
if err != nil {
@@ -400,59 +411,52 @@ func pkgSFiles(ctx *context, pkg *packages.Package) ([]string, error) {
400411
return v, nil
401412
}
402413

403-
// Fast path: if directory has no .s/.S at all, skip `go list`.
404-
if !dirHasAsmFile(pkg.Dir) {
405-
ctx.sfilesCache[pkg.ID] = nil
406-
return nil, nil
414+
var metadataSFiles []string
415+
for _, file := range pkg.OtherFiles {
416+
if strings.HasSuffix(file, ".s") || strings.HasSuffix(file, ".S") {
417+
metadataSFiles = append(metadataSFiles, file)
418+
}
407419
}
408-
409-
args := []string{"list", "-json"}
410-
if ctx.conf != nil && len(ctx.conf.BuildFlags) > 0 {
411-
args = append(args, ctx.conf.BuildFlags...)
420+
if len(metadataSFiles) > 0 {
421+
ctx.sfilesCache[pkg.ID] = metadataSFiles
422+
return metadataSFiles, nil
412423
}
413-
args = append(args, pkg.PkgPath)
414424

415-
cmd := exec.Command("go", args...)
416-
cmd.Dir = pkg.Dir
417-
cmd.Env = append(os.Environ(),
418-
"GOOS="+ctx.buildConf.Goos,
419-
"GOARCH="+ctx.buildConf.Goarch,
420-
)
421-
out, err := cmd.Output()
422-
if err != nil {
423-
var errBuf bytes.Buffer
424-
if ee, ok := err.(*exec.ExitError); ok && len(ee.Stderr) > 0 {
425-
errBuf.Write(ee.Stderr)
426-
}
427-
return nil, fmt.Errorf("go list -json %s failed: %w\n%s", pkg.PkgPath, err, strings.TrimSpace(errBuf.String()))
425+
// Fast path: if directory has no .s/.S at all, skip source selection.
426+
if !ctx.dirHasAsmFile(pkg.Dir) {
427+
ctx.sfilesCache[pkg.ID] = nil
428+
return nil, nil
428429
}
429430

430-
var lp struct {
431-
Dir string `json:"Dir"`
432-
SFiles []string `json:"SFiles"`
431+
buildCtx := build.Default
432+
buildCtx.GOOS = ctx.buildConf.Goos
433+
buildCtx.GOARCH = ctx.buildConf.Goarch
434+
if ctx.conf != nil {
435+
buildCtx.BuildTags = parseSourcePatchBuildTags(ctx.conf.BuildFlags)
433436
}
434-
if err := json.Unmarshal(out, &lp); err != nil {
435-
return nil, fmt.Errorf("go list -json %s: parse: %w", pkg.PkgPath, err)
437+
bp, err := buildCtx.ImportDir(pkg.Dir, 0)
438+
if err != nil {
439+
return nil, fmt.Errorf("inspect asm files for %s: %w", pkg.PkgPath, err)
436440
}
437441

438442
// internal/chacha8rand has highly optimized arch asm on amd64/arm64.
439443
// Until full vector lowering lands, force the generic stub entry, which
440444
// tail-jumps to block_generic and preserves package behavior.
441-
if pkg.PkgPath == "internal/chacha8rand" && lp.Dir != "" {
442-
stub := filepath.Join(lp.Dir, "chacha8_stub.s")
445+
if pkg.PkgPath == "internal/chacha8rand" && bp.Dir != "" {
446+
stub := filepath.Join(bp.Dir, "chacha8_stub.s")
443447
if _, err := os.Stat(stub); err == nil {
444448
paths := []string{stub}
445449
ctx.sfilesCache[pkg.ID] = paths
446450
return paths, nil
447451
}
448452
}
449453

450-
paths := make([]string, 0, len(lp.SFiles))
451-
for _, f := range lp.SFiles {
452-
if lp.Dir == "" {
454+
paths := make([]string, 0, len(bp.SFiles))
455+
for _, f := range bp.SFiles {
456+
if bp.Dir == "" {
453457
continue
454458
}
455-
paths = append(paths, filepath.Join(lp.Dir, f))
459+
paths = append(paths, filepath.Join(bp.Dir, f))
456460
}
457461
ctx.sfilesCache[pkg.ID] = paths
458462
return paths, nil

internal/build/plan9asm_altpkg_test.go

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,55 @@ func TestDirHasAsmFile(t *testing.T) {
3838
t.Fatal("directory with .S file should have asm files")
3939
}
4040
if !dirHasAsmFile(filepath.Join(t.TempDir(), "missing")) {
41-
t.Fatal("unreadable/missing directory should fall back to go list")
41+
t.Fatal("unreadable/missing directory should fall back to source selection")
42+
}
43+
}
44+
45+
func TestPkgSFilesUsesOtherFiles(t *testing.T) {
46+
dir := t.TempDir()
47+
pkg := &packages.Package{
48+
ID: "example.com/otherfiles",
49+
PkgPath: "example.com/otherfiles",
50+
Dir: dir,
51+
OtherFiles: []string{filepath.Join(dir, "selected.s"), filepath.Join(dir, "note.txt")},
52+
}
53+
ctx := &context{conf: &packages.Config{}, buildConf: &Config{Goos: "linux", Goarch: "amd64"}}
54+
files, err := pkgSFiles(ctx, pkg)
55+
if err != nil {
56+
t.Fatal(err)
57+
}
58+
want := filepath.Join(dir, "selected.s")
59+
if len(files) != 1 || files[0] != want {
60+
t.Fatalf("pkgSFiles returned %v, want [%s]", files, want)
61+
}
62+
if got := ctx.sfilesCache[pkg.ID]; len(got) != 1 || got[0] != want {
63+
t.Fatalf("sfiles cache = %v, want [%s]", got, want)
64+
}
65+
}
66+
67+
func TestPkgSFilesUsesBuildContextSelection(t *testing.T) {
68+
dir := t.TempDir()
69+
if err := os.WriteFile(filepath.Join(dir, "main.go"), []byte("package p\n"), 0o644); err != nil {
70+
t.Fatal(err)
71+
}
72+
if err := os.WriteFile(filepath.Join(dir, "selected_amd64.s"), []byte("TEXT ·selected(SB),$0-0\n"), 0o644); err != nil {
73+
t.Fatal(err)
74+
}
75+
if err := os.WriteFile(filepath.Join(dir, "skipped_arm64.s"), []byte("TEXT ·skipped(SB),$0-0\n"), 0o644); err != nil {
76+
t.Fatal(err)
77+
}
78+
ctx := &context{conf: &packages.Config{}, buildConf: &Config{Goos: "linux", Goarch: "amd64"}}
79+
pkg := &packages.Package{ID: "example.com/asmselect", PkgPath: "example.com/asmselect", Dir: dir}
80+
files, err := pkgSFiles(ctx, pkg)
81+
if err != nil {
82+
t.Fatal(err)
83+
}
84+
want := filepath.Join(dir, "selected_amd64.s")
85+
if len(files) != 1 || files[0] != want {
86+
t.Fatalf("pkgSFiles returned %v, want [%s]", files, want)
87+
}
88+
if has, ok := ctx.asmDirCache[dir]; !ok || !has {
89+
t.Fatalf("asm dir cache[%s] = %v, %v; want true", dir, has, ok)
4290
}
4391
}
4492

@@ -62,6 +110,9 @@ func TestPkgSFilesCachesNoAsmResult(t *testing.T) {
62110
if _, ok := ctx.sfilesCache[pkg.ID]; !ok {
63111
t.Fatal("no-asm result was not cached")
64112
}
113+
if has, ok := ctx.asmDirCache[dir]; !ok || has {
114+
t.Fatalf("asm dir cache[%s] = %v, %v; want false", dir, has, ok)
115+
}
65116
}
66117

67118
func TestHasAltPkgForTarget_AllowsAdditivePatchWithPlan9Asm(t *testing.T) {

0 commit comments

Comments
 (0)