diff --git a/src/cmd/compile/internal/ssa/_gen/RISCV64.rules b/src/cmd/compile/internal/ssa/_gen/RISCV64.rules index 504c75a2ee8b8f..dca1bccde0b02c 100644 --- a/src/cmd/compile/internal/ssa/_gen/RISCV64.rules +++ b/src/cmd/compile/internal/ssa/_gen/RISCV64.rules @@ -721,6 +721,14 @@ (SRLI [x] (MOVHUreg y)) && x >= 16 => (MOVDconst [0]) (SRLI [x] (MOVWUreg y)) && x >= 32 => (MOVDconst [0]) +// Remove unnecessary masking with shifts. +(SLL x (ANDI [63] y)) => (SLL x y) +(SRL x (ANDI [63] y)) => (SRL x y) +(SRA x (ANDI [63] y)) => (SRA x y) +(SLLW x (ANDI [31] y)) => (SLLW x y) +(SRLW x (ANDI [31] y)) => (SRLW x y) +(SRAW x (ANDI [31] y)) => (SRAW x y) + // Fold constant into immediate instructions where possible. (ADD (MOVDconst [val]) x) && ssa.Is32Bit(val) && !t.IsPtr() => (ADDI [val] x) (AND (MOVDconst [val]) x) && ssa.Is32Bit(val) => (ANDI [val] x) diff --git a/src/cmd/compile/internal/ssa/dfplus_iter.go b/src/cmd/compile/internal/ssa/dfplus_iter.go index 025cce6347182e..8784d6bc253584 100644 --- a/src/cmd/compile/internal/ssa/dfplus_iter.go +++ b/src/cmd/compile/internal/ssa/dfplus_iter.go @@ -18,16 +18,17 @@ import ( // IterDomFrontierPlus iterates the DF+ of seeds: every block at which // a phi may need to be placed if a variable were defined in the seed -// blocks. Blocks are yielded at most once, in a deterministic order; -// an early break stops the walk. Seed blocks themselves are not -// yielded as such, but a seed that is also a merge point (e.g. a loop -// header) is. +// blocks. For each frontier block, it also yields the block whose +// outgoing edge discovered the frontier. Frontier blocks are yielded +// at most once, in a deterministic order; an early break stops the walk. +// Seed blocks themselves are not yielded as such, but a seed that is +// also a merge point (e.g. a loop header) is. // seeds iterator is consumed in full before the walk starts (the current // algorithm has to walk deeper roots first). // CFG must not change while iteration is in progress; inserting // values (like phis) is fine. -func (f *Func) IterDomFrontierPlus(seeds iter.Seq[*Block]) iter.Seq[*Block] { - return func(yield func(*Block) bool) { +func (f *Func) IterDomFrontierPlus(seeds iter.Seq[*Block]) iter.Seq2[*Block, *Block] { + return func(yield func(*Block, *Block) bool) { // Materialize the seeds into a pooled slice reused by walkDFPlus. s := f.Cache.AllocBlockSlice(f.NumBlocks())[:0] defer f.Cache.FreeBlockSlice(s[:cap(s)]) @@ -61,7 +62,7 @@ const ( // plus the frontier found, and memory is O(f.NumBlocks()). The walk reads // the CFG's edges and uses the cached dominator tree. // The seeds slice is reused in place by the PiggyBank. -func (f *Func) walkDFPlus(seeds []*Block, yield func(*Block) bool) { +func (f *Func) walkDFPlus(seeds []*Block, yield func(*Block, *Block) bool) { sdom := f.Sdom() // Roots to process, deepest first. @@ -120,7 +121,7 @@ func (f *Func) walkDFPlus(seeds []*Block, yield func(*Block) bool) { flags[c.ID] |= flagPiggyBanked heap.Push(&piggyBank, c) } - if !yield(c) { + if !yield(c, b) { return } } diff --git a/src/cmd/compile/internal/ssa/dfplus_iter_test.go b/src/cmd/compile/internal/ssa/dfplus_iter_test.go index 8189df088137f9..439551cb4493d0 100644 --- a/src/cmd/compile/internal/ssa/dfplus_iter_test.go +++ b/src/cmd/compile/internal/ssa/dfplus_iter_test.go @@ -118,7 +118,29 @@ func TestIterDomFrontierPlusSeedAtMerge(t *testing.T) { } } -func collectBlockIDs(seq iter.Seq[*Block]) []ID { +func TestIterDomFrontierPlusOrigin(t *testing.T) { + f := (&Config{}).NewFunc(nil, &Cache{}) + entry := f.NewBlock(block.BlockIf) + f.Entry = entry + left := f.NewBlock(block.BlockPlain) + right := f.NewBlock(block.BlockPlain) + merge := f.NewBlock(block.BlockExit) + + entry.AddEdgeTo(left) + entry.AddEdgeTo(right) + left.AddEdgeTo(merge) + right.AddEdgeTo(merge) + + var got [][2]*Block + for b, origin := range f.IterDomFrontierPlus(slices.Values([]*Block{left})) { + got = append(got, [2]*Block{b, origin}) + } + if want := [][2]*Block{{merge, left}}; !slices.Equal(got, want) { + t.Fatalf("got frontier and origin %v, want %v", got, want) + } +} + +func collectBlockIDs(seq iter.Seq2[*Block, *Block]) []ID { var ids []ID for b := range seq { ids = append(ids, b.ID) diff --git a/src/cmd/compile/internal/ssacompile/cpufeatures.go b/src/cmd/compile/internal/ssacompile/cpufeatures.go index fd01675dadd92f..cccac88e6063bd 100644 --- a/src/cmd/compile/internal/ssacompile/cpufeatures.go +++ b/src/cmd/compile/internal/ssacompile/cpufeatures.go @@ -116,6 +116,17 @@ func ifEffect(b *ssa.Block) (features ssa.CPUfeatures, taken int) { return } +// noCodeValue reports whether v is known to emit no machine code, so +// that even a SIMD type does not imply the presence of any CPU feature. +func noCodeValue(v *ssa.Value) bool { + switch v.Op { + case ssaop.OpZeroSIMD, ssaop.OpPhi, ssaop.OpCopy, ssaop.OpSelectN, + ssaop.OpArg, ssaop.OpArgIntReg, ssaop.OpArgFloatReg: + return true + } + return false +} + func cpufeatures(f *ssa.Func) { arch := f.Config.Ctxt.Arch.Family // TODO there are other SIMD architectures @@ -183,6 +194,15 @@ func cpufeatures(f *ssa.Func) { // instruction that would fault if the feature (avx, avx512) // were not present, then assume that the feature is present // for all the instructions in the block, a fault is a fault. + if noCodeValue(v) { + // v emits no instruction, so its type implies + // nothing about the CPU. In particular a + // SIMD-typed zero is just a reference to the + // fixed all-zeros register and flows freely + // through code that must run on machines + // without AVX. + continue + } t := v.Type if t.IsResults() { for i := 0; i < t.NumFields(); i++ { diff --git a/src/cmd/compile/internal/ssacompile/critical.go b/src/cmd/compile/internal/ssacompile/critical.go index f4c3157e470fac..c700392d99c3ba 100644 --- a/src/cmd/compile/internal/ssacompile/critical.go +++ b/src/cmd/compile/internal/ssacompile/critical.go @@ -68,20 +68,39 @@ func critical(f *ssa.Func) { // the new blocks to be re-examined. d = f.NewBlock(block.BlockPlain) d.Pos = p.Pos + // CPU features are execution-invariant facts: d + // executes only after p and unconditionally jumps + // to b, so both blocks' features hold in d. + d.CPUfeatures = p.CPUfeatures | b.CPUfeatures blocks[argID] = d if f.Pass.Debug > 0 { f.Warnl(p.Pos, "split critical edge") + if d.CPUfeatures != ssa.CPUNone { + f.Warnl(p.Pos, "split-edge block b%d has features %v", d.ID, d.CPUfeatures) + } } } else { reusedBlock = true + // d gains another predecessor, so only the + // features common to all of its predecessors + // (plus b's) are still guaranteed. + d.CPUfeatures &= p.CPUfeatures | b.CPUfeatures + if f.Pass.Debug > 0 && d.CPUfeatures != ssa.CPUNone { + f.Warnl(p.Pos, "reused split-edge block b%d has features %v", d.ID, d.CPUfeatures) + } } } else { // no existing block, so allocate a new block // to place on the edge d = f.NewBlock(block.BlockPlain) d.Pos = p.Pos + // See above for why d inherits these features. + d.CPUfeatures = p.CPUfeatures | b.CPUfeatures if f.Pass.Debug > 0 { f.Warnl(p.Pos, "split critical edge") + if d.CPUfeatures != ssa.CPUNone { + f.Warnl(p.Pos, "split-edge block b%d has features %v", d.ID, d.CPUfeatures) + } } } diff --git a/src/cmd/compile/internal/ssagen/phi.go b/src/cmd/compile/internal/ssagen/phi.go index 6721f7df7d82b9..45e42bd03a276b 100644 --- a/src/cmd/compile/internal/ssagen/phi.go +++ b/src/cmd/compile/internal/ssagen/phi.go @@ -145,9 +145,9 @@ func (s *phiState) insertPhis() { func (s *phiState) insertVarPhis(n int, var_ ir.Node, defs []*ssa.Block, typ *types.Type) { // Iterate DF+ of the defining blocks. - for c := range s.f.IterDomFrontierPlus(slices.Values(defs)) { + for c, b := range s.f.IterDomFrontierPlus(slices.Values(defs)) { // Add a phi to block c for variable n. - v := c.NewValue0I(s.s.blockStarts[c.ID], ssaop.OpPhi, typ, int64(n)) + v := c.NewValue0I(s.s.blockStarts[b.ID], ssaop.OpPhi, typ, int64(n)) // Note: we store the variable number in the phi's AuxInt field. Used temporarily by phi building. if var_.Op() == ir.ONAME { s.s.addNamedValue(var_.(*ir.Name), v) diff --git a/src/cmd/compile/internal/ssarewrite/rewriteriscv64/rewriteRISCV64.go b/src/cmd/compile/internal/ssarewrite/rewriteriscv64/rewriteRISCV64.go index 7c6b165f51cda5..5353d78bc01702 100644 --- a/src/cmd/compile/internal/ssarewrite/rewriteriscv64/rewriteRISCV64.go +++ b/src/cmd/compile/internal/ssarewrite/rewriteriscv64/rewriteRISCV64.go @@ -8311,6 +8311,18 @@ func rewriteValue_OpRISCV64SEQZ(v *ssa.Value) bool { func rewriteValue_OpRISCV64SLL(v *ssa.Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] + // match: (SLL x (ANDI [63] y)) + // result: (SLL x y) + for { + x := v_0 + if v_1.Op != ssaop.OpRISCV64ANDI || ssa.AuxIntToInt64(v_1.AuxInt) != 63 { + break + } + y := v_1.Args[0] + v.Reset(ssaop.OpRISCV64SLL) + v.AddArg2(x, y) + return true + } // match: (SLL x (MOVDconst [val])) // result: (SLLI [val&63] x) for { @@ -8384,6 +8396,18 @@ func rewriteValue_OpRISCV64SLLI(v *ssa.Value) bool { func rewriteValue_OpRISCV64SLLW(v *ssa.Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] + // match: (SLLW x (ANDI [31] y)) + // result: (SLLW x y) + for { + x := v_0 + if v_1.Op != ssaop.OpRISCV64ANDI || ssa.AuxIntToInt64(v_1.AuxInt) != 31 { + break + } + y := v_1.Args[0] + v.Reset(ssaop.OpRISCV64SLLW) + v.AddArg2(x, y) + return true + } // match: (SLLW x (MOVDconst [val])) // result: (SLLIW [val&31] x) for { @@ -8637,6 +8661,18 @@ func rewriteValue_OpRISCV64SNEZ(v *ssa.Value) bool { func rewriteValue_OpRISCV64SRA(v *ssa.Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] + // match: (SRA x (ANDI [63] y)) + // result: (SRA x y) + for { + x := v_0 + if v_1.Op != ssaop.OpRISCV64ANDI || ssa.AuxIntToInt64(v_1.AuxInt) != 63 { + break + } + y := v_1.Args[0] + v.Reset(ssaop.OpRISCV64SRA) + v.AddArg2(x, y) + return true + } // match: (SRA x (MOVDconst [val])) // result: (SRAI [val&63] x) for { @@ -8748,6 +8784,18 @@ func rewriteValue_OpRISCV64SRAI(v *ssa.Value) bool { func rewriteValue_OpRISCV64SRAW(v *ssa.Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] + // match: (SRAW x (ANDI [31] y)) + // result: (SRAW x y) + for { + x := v_0 + if v_1.Op != ssaop.OpRISCV64ANDI || ssa.AuxIntToInt64(v_1.AuxInt) != 31 { + break + } + y := v_1.Args[0] + v.Reset(ssaop.OpRISCV64SRAW) + v.AddArg2(x, y) + return true + } // match: (SRAW x (MOVDconst [val])) // result: (SRAIW [val&31] x) for { @@ -8766,6 +8814,18 @@ func rewriteValue_OpRISCV64SRAW(v *ssa.Value) bool { func rewriteValue_OpRISCV64SRL(v *ssa.Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] + // match: (SRL x (ANDI [63] y)) + // result: (SRL x y) + for { + x := v_0 + if v_1.Op != ssaop.OpRISCV64ANDI || ssa.AuxIntToInt64(v_1.AuxInt) != 63 { + break + } + y := v_1.Args[0] + v.Reset(ssaop.OpRISCV64SRL) + v.AddArg2(x, y) + return true + } // match: (SRL x (MOVDconst [val])) // result: (SRLI [val&63] x) for { @@ -8862,6 +8922,18 @@ func rewriteValue_OpRISCV64SRLI(v *ssa.Value) bool { func rewriteValue_OpRISCV64SRLW(v *ssa.Value) bool { v_1 := v.Args[1] v_0 := v.Args[0] + // match: (SRLW x (ANDI [31] y)) + // result: (SRLW x y) + for { + x := v_0 + if v_1.Op != ssaop.OpRISCV64ANDI || ssa.AuxIntToInt64(v_1.AuxInt) != 31 { + break + } + y := v_1.Args[0] + v.Reset(ssaop.OpRISCV64SRLW) + v.AddArg2(x, y) + return true + } // match: (SRLW x (MOVDconst [val])) // result: (SRLIW [val&31] x) for { diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index 5632b224d801f5..4caea619a35a73 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -2228,19 +2228,19 @@ func TestTestCache(t *testing.T) { // Changing the actual package should have limited effects. tg.tempFile("src/p1/p1.go", "package p1\nvar X = 02\n") tg.run("test", "-p=1", "-x", "-v", "-short", "t/...") - - // p2 should have been rebuilt. - tg.grepStderr(`([\\/]compile|gccgo).*p2.go`, "did not recompile p2") + // p2 should not have been rebuilt. + tg.grepStderrNot(`([\\/]compile|gccgo).*p2.go`, "incorrectly recompiled p2") // t1 does not import anything, should not have been rebuilt. tg.grepStderrNot(`([\\/]compile|gccgo).*t1_test.go`, "incorrectly recompiled t1") tg.grepStderrNot(`([\\/]link|gccgo).*t1_test`, "incorrectly relinked t1_test") tg.grepStdout(`ok \tt/t1\t\(cached\)`, "did not cache t/t1") - // t2 imports p1 and must be rebuilt and relinked, - // but the change should not have any effect on the test binary, + // t2 imports p1 and it must not be rebuilt because p1's export data + // didn't change but must be relinked because p1's object data did. + // The change should not have any effect on the test binary, // so the test should not have been rerun. - tg.grepStderr(`([\\/]compile|gccgo).*t2_test.go`, "did not recompile t2") + tg.grepStderrNot(`([\\/]compile|gccgo).*t2_test.go`, "incorrectly recompiled t2") tg.grepStderr(`([\\/]link|gccgo).*t2\.test`, "did not relink t2_test") // This check does not currently work with gccgo, as garbage // collection of unused variables is not turned on by default. @@ -2249,7 +2249,7 @@ func TestTestCache(t *testing.T) { } // t3 imports p1, and changing X changes t3's test binary. - tg.grepStderr(`([\\/]compile|gccgo).*t3_test.go`, "did not recompile t3") + tg.grepStderrNot(`([\\/]compile|gccgo).*t3_test.go`, "incorrectly recompiled t3") tg.grepStderr(`([\\/]link|gccgo).*t3\.test`, "did not relink t3_test") tg.grepStderr(`t3\.test.*-test.short`, "did not rerun t3_test") tg.grepStdoutNot(`ok \tt/t3\t\(cached\)`, "reported cached t3_test result") diff --git a/src/cmd/go/internal/work/action.go b/src/cmd/go/internal/work/action.go index 65e4d324d3a4d4..9c7dacb76e7659 100644 --- a/src/cmd/go/internal/work/action.go +++ b/src/cmd/go/internal/work/action.go @@ -128,10 +128,10 @@ type Action struct { } // BuildActionID returns the action ID section of a's build ID. -func (a *Action) BuildActionID() string { return actionID(a.buildID) } +func (a *Action) BuildActionID() string { return buildActionID(a.buildID) } // BuildContentID returns the content ID section of a's build ID. -func (a *Action) BuildContentID() string { return contentID(a.buildID) } +func (a *Action) BuildContentID() string { return buildObjectID(a.buildID) } // BuildID returns a's build ID. func (a *Action) BuildID() string { return a.buildID } diff --git a/src/cmd/go/internal/work/buildid.go b/src/cmd/go/internal/work/buildid.go index 573a6da38867fa..8c517703c27fc4 100644 --- a/src/cmd/go/internal/work/buildid.go +++ b/src/cmd/go/internal/work/buildid.go @@ -6,7 +6,9 @@ package work import ( "bytes" + "cmd/internal/archive" "fmt" + "io" "os" "os/exec" "strings" @@ -27,37 +29,41 @@ import ( // // Go packages and binaries are stamped with build IDs that record both // the action ID, which is a hash of the inputs to the action that produced -// the packages or binary, and the content ID, which is a hash of the action -// output, namely the archive or binary itself. The hash is the same one +// the packages or binary, and their content IDs which are hashes of the +// action outputs. The content IDs are hashes of the export and object files +// in the case of gc package builds, or of the entire archive or binary, in the case +// of gccgo builds or link actions. These hashes are the same // used by the build artifact cache (see cmd/go/internal/cache), but // truncated when stored in packages and binaries, as the full length is not -// needed and is a bit unwieldy. The precise form is +// needed and is a bit unwieldy. The precise forms are // -// actionID/[.../]contentID +// actionID/contentID(export)/contentID(object) +// actionID(link)/actionID(build)/contentID(build object)/contentID(link) // -// where the actionID and contentID are prepared by buildid.HashToString below. +// where the first form is used for build actions and the second form is used +// for link actions. The actionID and contentID are prepared by buildid.HashToString below. // and are found by looking for the first or last slash. -// Usually the buildID is simply actionID/contentID, but see below for an -// exception. +// gccgo actions use the same gccgo output for the export and object +// content ids because they do not have separate export data files. // // The build ID serves two primary purposes. // -// 1. The action ID half allows installed packages and binaries to serve as +// 1. The action ID part allows installed packages and binaries to serve as // one-element cache entries. If we intend to build math.a with a given // set of inputs summarized in the action ID, and the installed math.a already // has that action ID, we can reuse the installed math.a instead of rebuilding it. // -// 2. The content ID half allows the easy preparation of action IDs for steps -// that consume a particular package or binary. The content hash of every -// input file for a given action must be included in the action ID hash. -// Storing the content ID in the build ID lets us read it from the file with -// minimal I/O, instead of reading and hashing the entire file. -// This is especially effective since packages and binaries are typically +// 2. The content ID parts allow the easy preparation of action IDs for steps +// that consume a particular package's export or object files or its binary. +// The content hash of every input file for a given action must be included +// in the action ID hash. Storing the content IDs in the build ID lets us read +// it from the file with minimal I/O, instead of reading and hashing the entire +// file. This is especially effective since packages and binaries are typically // the largest inputs to an action. // -// Separating action ID from content ID is important for reproducible builds. +// Separating action ID from content IDs is important for reproducible builds. // The compiler is compiled with itself. If an output were represented by its -// own action ID (instead of content ID) when computing the action ID of +// own action ID (instead of content IDs) when computing the action ID of // the next step in the build process, then the compiler could never have its // own input action ID as its output action ID (short of a miraculous hash collision). // Instead we use the content IDs to compute the next action ID, and because @@ -75,20 +81,19 @@ import ( // means knowing the content ID of main.a, which we did not keep. // To sidestep this problem, each binary actually stores an expanded build ID: // -// actionID(binary)/actionID(main.a)/contentID(main.a)/contentID(binary) +// actionID(binary)/actionID(main.a)/contentID(main.a object)/contentID(binary) // -// (Note that this can be viewed equivalently as: -// -// actionID(binary)/buildID(main.a)/contentID(binary) -// -// Storing the buildID(main.a) in the middle lets the computations that care -// about the prefix or suffix halves ignore the middle and preserves the -// original build ID as a contiguous string.) +// where contentID(main.a object) only includes the hash of main.a's object +// files but not its export data. +// Storing the action and object content ids of the build action in the middle +// lets the computations that care about the prefix or suffix halves ignore the middle, +// while keeping the action and output info needed to check that the build +// action does not need to be rerun. // // During the build, when it's time to build main.a, the gofmt binary has the // information needed to decide whether the eventual link would produce // the same binary: if the action ID for main.a's inputs matches and then -// the action ID for the link step matches when assuming the given main.a +// the action ID for the link step matches when assuming the given main.a's object's // content ID, then the binary as a whole is up-to-date and need not be rebuilt. // // This is all a bit complex and may be simplified once we can rely on the @@ -98,8 +103,8 @@ import ( const buildIDSeparator = "/" -// actionID returns the action ID half of a build ID. -func actionID(buildID string) string { +// buildActionID returns the action ID part of a build ID. +func buildActionID(buildID string) string { i := strings.Index(buildID, buildIDSeparator) if i < 0 { return buildID @@ -107,11 +112,20 @@ func actionID(buildID string) string { return buildID[:i] } -// contentID returns the content ID half of a build ID. -func contentID(buildID string) string { +// buildObjectID returns the content ID for the object data. +func buildObjectID(buildID string) string { return buildID[strings.LastIndex(buildID, buildIDSeparator)+1:] } +// buildExportID returns the content ID for the export data. +func buildExportID(buildID string) string { + if buildID == "" { + return "" + } + chopContent := buildID[:strings.LastIndex(buildID, buildIDSeparator)] + return chopContent[strings.LastIndex(chopContent, buildIDSeparator)+1:] +} + // toolID returns the unique ID to use for the current copy of the // named tool (asm, compile, cover, link). // @@ -175,7 +189,7 @@ func (b *Builder) toolID(name string) string { } if strings.Contains(f[2], "devel") { // On the development branch, use the content ID part of the build ID. - return contentID(f[len(f)-1]) + return buildObjectID(f[len(f)-1]) } // For a release, the output is like: "compile version go1.9.1 X:framepointer". // Use the whole line. @@ -464,13 +478,13 @@ func (b *Builder) useCache(a *Action, actionHash cache.ActionID, target string, a.json.ActionID = actionID } contentID := actionID // temporary placeholder, likely unique - a.buildID = actionID + buildIDSeparator + contentID + a.buildID = actionID + buildIDSeparator + contentID + buildIDSeparator + contentID - // Executable binaries also record the main build ID in the middle. + // Executable binaries also record the action and object content id of the build id in the middle. // See "Build IDs" comment above. if a.Mode == "link" { mainpkg := a.Deps[0] - a.buildID = actionID + buildIDSeparator + mainpkg.buildID + buildIDSeparator + contentID + a.buildID = actionID + buildIDSeparator + buildActionID(mainpkg.buildID) + buildIDSeparator + buildObjectID(mainpkg.buildID) + buildIDSeparator + contentID } // If user requested -a, we force a rebuild, so don't use the cache. @@ -530,7 +544,7 @@ func (b *Builder) useCache(a *Action, actionHash cache.ActionID, target string, // other than a.buildID, b.linkActionID is only accessing // build IDs of completed actions. oldBuildID := a.buildID - a.buildID = id[1] + buildIDSeparator + id[2] + a.buildID = id[1] + buildIDSeparator + id[2] + buildIDSeparator + id[2] linkID := buildid.HashToString(b.linkActionID(a.triggers[0])) if id[0] == linkID { // Best effort attempt to display output from the compile and link steps. @@ -707,17 +721,52 @@ func (b *Builder) updateBuildID(a *Action, target string) error { } } + var matches []int64 + var exportHash [32]byte + var objectOffset int64 // where to start hashing the object data from // Find occurrences of old ID and compute new content-based ID. r, err := os.Open(target) if err != nil { return err } - matches, hash, err := buildid.FindAndHash(r, a.buildID, 0) - r.Close() + // Hash export id if this is an archive with an export data file. + if v, err := archive.Parse(r, false); err == nil && len(v.Entries) > 0 && v.Entries[0].Type == archive.EntryPkgDef { + pkgEntry := v.Entries[0] + exportMatches, contentHash, err := buildid.FindAndHash(io.NewSectionReader(r, pkgEntry.Offset, pkgEntry.Size), a.buildID, 0) + if err != nil { + r.Close() + return err + } + exportHash = contentHash + for _, m := range exportMatches { + matches = append(matches, pkgEntry.Offset+m) + } + objectOffset = pkgEntry.Offset + pkgEntry.Size + } + if _, err := r.Seek(objectOffset, io.SeekStart); err != nil { + r.Close() + return err + } + objectMatches, objectHash, err := buildid.FindAndHash(r, a.buildID, 0) if err != nil { return err } - newID := a.buildID[:strings.LastIndex(a.buildID, buildIDSeparator)] + buildIDSeparator + buildid.HashToString(hash) + for _, m := range objectMatches { + matches = append(matches, objectOffset+m) + } + if err := r.Close(); err != nil { + return err + } + + var newID string + if a.Mode == "build" { + if exportHash == [32]byte{} { + exportHash = objectHash // gccgo does not have export data + } + newID = buildActionID(a.buildID) + buildIDSeparator + buildid.HashToString(exportHash) + buildIDSeparator + buildid.HashToString(objectHash) + } else { + newID = a.buildID[:strings.LastIndex(a.buildID, buildIDSeparator)] + buildIDSeparator + buildid.HashToString(objectHash) + } if len(newID) != len(a.buildID) { return fmt.Errorf("internal error: build ID length mismatch %q vs %q", a.buildID, newID) } diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go index e61d891d6cfebe..c711a906202495 100644 --- a/src/cmd/go/internal/work/exec.go +++ b/src/cmd/go/internal/work/exec.go @@ -373,7 +373,7 @@ func (b *Builder) buildActionID(a *Action) cache.ActionID { for _, a1 := range a.Deps { p1 := a1.Package if p1 != nil && p1 != p { // p can show up in its own action deps in a cache or cgo action - fmt.Fprintf(h, "import %s %s\n", p1.ImportPath, contentID(a1.buildID)) + fmt.Fprintf(h, "import %s %s\n", p1.ImportPath, buildExportID(a1.buildID)) } if a1.Mode == "preprocess PGO profile" { fmt.Fprintf(h, "pgofile %s\n", b.fileHash(a1.built)) @@ -1868,7 +1868,7 @@ func (b *Builder) exportActionID(a *Action, ecfg *exportConfig) cache.ActionID { } // Any dependencies. for _, dep := range a.Deps { - fmt.Fprintf(h, "packageFile %s=%s\n", dep.Package.ImportPath, contentID(dep.buildID)) + fmt.Fprintf(h, "packageFile %s=%s\n", dep.Package.ImportPath, buildExportID(dep.buildID)) } return cache.ActionID(h.Sum()) } @@ -1915,15 +1915,17 @@ func (b *Builder) linkActionID(a *Action) cache.ActionID { if buildID == "" { buildID = b.buildID(a1.built) } - fmt.Fprintf(h, "packagefile %s=%s\n", p1.ImportPath, contentID(buildID)) + fmt.Fprintf(h, "packagefile %s=%s\n", p1.ImportPath, buildObjectID(buildID)) } - // Because we put package main's full action ID into the binary's build ID, - // we must also put the full action ID into the binary's action ID hash. + // Because we put package main's action ID and object data content ID into the binary's build ID, + // we must also put the action ID and object data content ID into the binary's action ID hash. + // We only put in the object data content ID, which was hashed from everything other than the export data, + // and not the export data content ID, because the export data is not given to the linker. if p1.Name == "main" { - fmt.Fprintf(h, "packagemain %s\n", a1.buildID) + fmt.Fprintf(h, "packagemain %s\n", buildActionID(a1.buildID)+buildIDSeparator+buildObjectID(a1.buildID)) } if p1.Shlib != "" { - fmt.Fprintf(h, "packageshlib %s=%s\n", p1.ImportPath, contentID(b.buildID(p1.Shlib))) + fmt.Fprintf(h, "packageshlib %s=%s\n", p1.ImportPath, buildObjectID(b.buildID(p1.Shlib))) } } } @@ -2248,16 +2250,16 @@ func (b *Builder) linkSharedActionID(a *Action) cache.ActionID { continue } if p1 != nil { - fmt.Fprintf(h, "packagefile %s=%s\n", p1.ImportPath, contentID(b.buildID(a1.built))) + fmt.Fprintf(h, "packagefile %s=%s\n", p1.ImportPath, buildObjectID(b.buildID(a1.built))) if p1.Shlib != "" { - fmt.Fprintf(h, "packageshlib %s=%s\n", p1.ImportPath, contentID(b.buildID(p1.Shlib))) + fmt.Fprintf(h, "packageshlib %s=%s\n", p1.ImportPath, buildObjectID(b.buildID(p1.Shlib))) } } } // Files named on command line are special. for _, a1 := range a.Deps[0].Deps { p1 := a1.Package - fmt.Fprintf(h, "top %s=%s\n", p1.ImportPath, contentID(b.buildID(a1.built))) + fmt.Fprintf(h, "top %s=%s\n", p1.ImportPath, buildObjectID(b.buildID(a1.built))) } return h.Sum() diff --git a/src/cmd/go/internal/work/gc.go b/src/cmd/go/internal/work/gc.go index fa60d533bd1cde..ec198b6469dee2 100644 --- a/src/cmd/go/internal/work/gc.go +++ b/src/cmd/go/internal/work/gc.go @@ -583,14 +583,14 @@ func pluginPath(a *Action) string { // For linking, use the main package's build ID instead of // the binary's build ID, so it is the same hash used in // compiling and linking. - // When compiling, we use actionID/actionID (instead of - // actionID/contentID) as a temporary build ID to compute + // When compiling, we use actionID/actionID/actionID, instead of + // actionID/contentID(export)/contentID(object), as a temporary build ID to compute // the hash. Do the same here. (See buildid.go:useCache) // The build ID matters because it affects the overall hash // in the plugin's pseudo-import path returned below. // We need to use the same import path when compiling and linking. id := strings.Split(buildID, buildIDSeparator) - buildID = id[1] + buildIDSeparator + id[1] + buildID = id[1] + buildIDSeparator + id[1] + buildIDSeparator + id[1] } fmt.Fprintf(h, "build ID: %s\n", buildID) for _, file := range str.StringList(p.GoFiles, p.CgoFiles, p.SFiles) { diff --git a/src/cmd/go/testdata/script/cgo_path_space_quote.txt b/src/cmd/go/testdata/script/cgo_path_space_quote.txt index b3448e71f755f6..2dcafeeec1e527 100644 --- a/src/cmd/go/testdata/script/cgo_path_space_quote.txt +++ b/src/cmd/go/testdata/script/cgo_path_space_quote.txt @@ -4,7 +4,7 @@ # with single or double quotes. This is the same as -gcflags and similar # options. -[!exec:clang] [!exec:gcc] skip +[!cc:clang] [!cc:gcc] skip [!cgo] skip [GOARCH:ppc64le] skip 'broken on ppc64le: #81174' [GOARCH:ppc64] skip 'broken on ppc64: #81174' @@ -12,8 +12,8 @@ env GOENV=$WORK/go.env mkdir 'program files' go build -o 'program files' './which cc/which cc.go' -[exec:clang] env CC='"'$PWD${/}program' 'files${/}which' 'cc"' 'clang -[!exec:clang] env CC='"'$PWD${/}program' 'files${/}which' 'cc"' 'gcc +[cc:clang] env CC='"'$PWD${/}program' 'files${/}which' 'cc"' 'clang +[cc:gcc] env CC='"'$PWD${/}program' 'files${/}which' 'cc"' 'gcc go env CC stdout 'program files[/\\]which cc" (clang|gcc)$' go env -w CC=$CC diff --git a/src/cmd/go/testdata/script/cover_list.txt b/src/cmd/go/testdata/script/cover_list.txt index 7d723490f6c0a1..f7694576d841a3 100644 --- a/src/cmd/go/testdata/script/cover_list.txt +++ b/src/cmd/go/testdata/script/cover_list.txt @@ -15,7 +15,7 @@ stale -cover m/example # Collect build ID from for m/example built with -cover. go list -cover -export -f '{{.BuildID}}' m/example -cp stdout $WORK/listbuildid.txt +cp stdout $WORK/rawlistbuildid.txt # Now build the m/example binary with coverage. go build -cover -o $WORK/m.exe m/example @@ -26,9 +26,11 @@ cp stdout $WORK/rawtoolbuildid.txt # Make sure that the two build IDs agree with respect to the # m/example package. Build IDs from binaries are of the form X/Y/Z/W -# where Y/Z is the package build ID; running the program below will -# pick out the parts of the ID that we want. +# where Y is the package action ID and Z is the package's object's content ID. +# Running the program below will pick out the parts of the ID that we want. env GOCOVERDIR=$WORK +exec $WORK/m.exe $WORK/rawlistbuildid.txt +cp stdout $WORK/listbuildid.txt exec $WORK/m.exe $WORK/rawtoolbuildid.txt cp stdout $WORK/toolbuildid.txt @@ -59,8 +61,16 @@ func main() { os.Exit(1) } fields := strings.Split(strings.TrimSpace(string(content)), "/") - if len(fields) != 4 { - os.Exit(2) + switch len(fields) { + case 3: + // We're reading a build action id X/Y/Z: return X/Z, the + // part that will be embedded in the link's build id. + fmt.Println(fields[0] + "/" + fields[2]) + case 4: + // We're reading a link action id X/Y/Z/W: return Y/Z the part + // derived from the main package's build action. + fmt.Println(fields[1] + "/" + fields[2]) + default: + os.Exit(2) } - fmt.Println(fields[1] + "/" + fields[2]) } diff --git a/src/cmd/go/testdata/script/goauth_git.txt b/src/cmd/go/testdata/script/goauth_git.txt index 94b31b4543e45b..8ec66ec1428486 100644 --- a/src/cmd/go/testdata/script/goauth_git.txt +++ b/src/cmd/go/testdata/script/goauth_git.txt @@ -8,7 +8,9 @@ env GOSUMDB=off # Disable 'git credential fill' interactive prompts. env GIT_TERMINAL_PROMPT=0 exec git init -exec git config credential.helper 'store --file=.git-credentials' +# Ignore credential helpers inherited from system configuration. +exec git config credential.helper '' +exec git config --add credential.helper 'store --file=.git-credentials' cp go.mod.orig go.mod # Set GOAUTH to git without a working directory. diff --git a/src/math/big/int.go b/src/math/big/int.go index e2630c000deb84..68a178582f1221 100644 --- a/src/math/big/int.go +++ b/src/math/big/int.go @@ -403,7 +403,7 @@ func (z *Int) Divide(x, y, r *Int, mode RoundingMode) (*Int, *Int) { r_abs = r.abs } y_abs := y.abs // save y - if z == y || alias(z_abs, y.abs) { + if z == y || r == y || alias(z_abs, y.abs) || alias(r_abs, y.abs) { y_abs = nat(nil).set(y.abs) } neg := x.neg != y.neg diff --git a/src/math/big/int_test.go b/src/math/big/int_test.go index 4b5261c3de6401..6339336371f444 100644 --- a/src/math/big/int_test.go +++ b/src/math/big/int_test.go @@ -573,6 +573,56 @@ func TestIntDivide(t *testing.T) { } } +func TestIntDivideRemainderAliasingDivisor(t *testing.T) { + tests := []struct { + name string + x, y, q, r int64 + mode RoundingMode + }{ + {"trunc", 5, 3, 1, 2, Trunc}, + {"ceil", 5, 3, 2, -1, Ceil}, + {"floor", -5, 3, -2, 1, Floor}, + {"round", 2, 3, 1, -1, Round}, + } + + for _, test := range tests { + for _, scaleString := range []string{"1", "12345678901234567890"} { + t.Run(test.name+"/scale="+scaleString, func(t *testing.T) { + scale, ok := new(Int).SetString(scaleString, 10) + if !ok { + t.Fatal("invalid test scale") + } + x := new(Int).Mul(NewInt(test.x), scale) + y := new(Int).Mul(NewInt(test.y), scale) + wantQ := NewInt(test.q) + wantR := new(Int).Mul(NewInt(test.r), scale) + + gotQ, gotR := new(Int).Divide(x, y, y, test.mode) + if gotQ.Cmp(wantQ) != 0 || gotR.Cmp(wantR) != 0 { + t.Fatalf("Divide(%v, %v, y, %v) = (%v, %v); want (%v, %v)", x, test.y, test.mode, gotQ, gotR, wantQ, wantR) + } + + y.Mul(NewInt(test.y), scale) + _, gotR = (*Int)(nil).Divide(x, y, y, test.mode) + if gotR.Cmp(wantR) != 0 { + t.Fatalf("Divide(%v, %v, y, %v) with nil quotient returned remainder %v; want %v", x, test.y, test.mode, gotR, wantR) + } + }) + } + } + + t.Run("shared backing array", func(t *testing.T) { + x := NewInt(5) + y := NewInt(3) + r := new(Int).SetBits(y.Bits()) + + gotQ, gotR := new(Int).Divide(x, y, r, Ceil) + if gotQ.Cmp(NewInt(2)) != 0 || gotR.Cmp(NewInt(-1)) != 0 { + t.Fatalf("Divide(5, 3, r, Ceil) = (%v, %v); want (2, -1)", gotQ, gotR) + } + }) +} + var bitLenTests = []struct { in string out int diff --git a/src/net/http/internal/http2/server.go b/src/net/http/internal/http2/server.go index c6d27d96146cca..addfc1dd24ffa3 100644 --- a/src/net/http/internal/http2/server.go +++ b/src/net/http/internal/http2/server.go @@ -33,6 +33,7 @@ import ( "crypto/tls" "errors" "fmt" + "internal/synctest" "io" "log" "math" @@ -103,10 +104,6 @@ var ( type Server struct { mu sync.Mutex activeConns map[*serverConn]struct{} - - // Pool of error channels. This is per-Server rather than global - // because channels can't be reused across synctest bubbles. - errChanPool sync.Pool } func (s *Server) registerConn(sc *serverConn) { @@ -138,30 +135,33 @@ func (s *Server) startGracefulShutdown() { s.mu.Unlock() } -// Global error channel pool used for uninitialized Servers. -// We use a per-Server pool when possible to avoid using channels across synctest bubbles. +// errChanPool is a pool of reusable channels for reporting the result +// of a blocking frame write. +// +// The pool is not used inside synctest bubbles, since a channel created +// in one bubble can't be used from another bubble or from outside a +// bubble, and sync.Pool is not bubble-aware. var errChanPool = sync.Pool{ New: func() any { return make(chan error, 1) }, } -func (s *Server) getErrChan() chan error { - if s == nil { - return errChanPool.Get().(chan error) // Server used without calling ConfigureServer +func getErrChan() chan error { + if synctest.IsInBubble() { + // Channels can't be shared across synctest bubbles. + // Skip the pool; allocation cost is irrelevant in tests. + return make(chan error, 1) } - return s.errChanPool.Get().(chan error) + return errChanPool.Get().(chan error) } -func (s *Server) putErrChan(ch chan error) { - if s == nil { - errChanPool.Put(ch) // Server used without calling ConfigureServer - return +func putErrChan(ch chan error) { + if !synctest.IsInBubble() { + errChanPool.Put(ch) } - s.errChanPool.Put(ch) } func (s *Server) Configure(conf ServerConfig, tcfg *tls.Config) error { s.activeConns = make(map[*serverConn]struct{}) - s.errChanPool = sync.Pool{New: func() any { return make(chan error, 1) }} if tcfg.CipherSuites != nil && tcfg.MinVersion < tls.VersionTLS13 { // If they already provided a TLS 1.0–1.2 CipherSuite list, return an @@ -1009,7 +1009,7 @@ var writeDataPool = sync.Pool{ // writeDataFromHandler writes DATA response frames from a handler on // the given stream. func (sc *serverConn) writeDataFromHandler(stream *stream, data []byte, endStream bool) error { - ch := sc.srv.getErrChan() + ch := getErrChan() writeArg := writeDataPool.Get().(*writeData) *writeArg = writeData{stream.id, data, endStream} err := sc.writeFrameFromHandler(FrameWriteRequest{ @@ -1041,7 +1041,7 @@ func (sc *serverConn) writeDataFromHandler(stream *stream, data []byte, endStrea return errStreamClosed } } - sc.srv.putErrChan(ch) + putErrChan(ch) if frameWriteDone { writeDataPool.Put(writeArg) } @@ -2353,7 +2353,7 @@ func (sc *serverConn) writeHeaders(st *stream, headerData *writeResHeaders) erro // waiting for this frame to be written, so an http.Flush mid-handler // writes out the correct value of keys, before a handler later potentially // mutates it. - errc = sc.srv.getErrChan() + errc = getErrChan() } if err := sc.writeFrameFromHandler(FrameWriteRequest{ write: headerData, @@ -2365,7 +2365,7 @@ func (sc *serverConn) writeHeaders(st *stream, headerData *writeResHeaders) erro if errc != nil { select { case err := <-errc: - sc.srv.putErrChan(errc) + putErrChan(errc) return err case <-sc.doneServing: return errClientDisconnected @@ -3063,7 +3063,7 @@ func (w *responseWriter) Push(target, method string, header Header) error { method: method, url: u, header: cloneHeader(header), - done: sc.srv.getErrChan(), + done: getErrChan(), } select { @@ -3080,7 +3080,7 @@ func (w *responseWriter) Push(target, method string, header Header) error { case <-st.cw: return errStreamClosed case err := <-msg.done: - sc.srv.putErrChan(msg.done) + putErrChan(msg.done) return err } } diff --git a/src/runtime/testdata/testprogcgo/notingo.go b/src/runtime/testdata/testprogcgo/notingo.go index a385ae24d6fb34..7b68d686803ce8 100644 --- a/src/runtime/testdata/testprogcgo/notingo.go +++ b/src/runtime/testdata/testprogcgo/notingo.go @@ -64,8 +64,36 @@ import ( "runtime" "runtime/metrics" "sync/atomic" + "time" ) +// waitNotInGo waits for /sched/goroutines/not-in-go to read want, and reports +// whether it got there. +// +// A single read of the metric can legitimately disagree with want. It's +// documented as an approximate count, and while some other thread is partway +// through taking a P away from a goroutine in a cgo call, that goroutine is +// briefly counted in neither half of the runtime's accounting, so the reading +// comes up one short. The skew lasts only as long as the handoff, so reading +// again converges. See go.dev/issue/78877. +// +// The accounting bugs this program exists to catch make the count wrong and +// keep it wrong, which never converges and still fails here. +func waitNotInGo(what string, want uint64) bool { + s := []metrics.Sample{{Name: "/sched/goroutines/not-in-go:goroutines"}} + var n uint64 + for start := time.Now(); time.Since(start) < 5*time.Second; { + metrics.Read(s) + n = s[0].Value.Uint64() + if n == want { + return true + } + time.Sleep(time.Millisecond) + } + println(what, "expected", want, "not-in-go goroutines, found", n) + return false +} + func init() { register("NotInGoMetricCgoCall", NotInGoMetricCgoCall) register("NotInGoMetricCgoCallback", NotInGoMetricCgoCallback) @@ -88,12 +116,7 @@ func NotInGoMetricCgoCall() { } // Read not-in-go before taking the Ps back. - s := []metrics.Sample{{Name: "/sched/goroutines/not-in-go:goroutines"}} - failed := false - metrics.Read(s) - if n := s[0].Value.Uint64(); n != N { - println("pre-STW: expected", N, "not-in-go goroutines, found", n) - } + failed := !waitNotInGo("pre-STW:", N) // Do something that stops the world to take all the Ps back. // @@ -102,9 +125,8 @@ func NotInGoMetricCgoCall() { runtime.ReadMemStats(&m) // Read not-in-go. - metrics.Read(s) - if n := s[0].Value.Uint64(); n != N { - println("post-STW: expected", N, "not-in-go goroutines, found", n) + if !waitNotInGo("post-STW:", N) { + failed = true } // Fail if we get a bad reading. @@ -153,10 +175,7 @@ func NotInGoMetricCgoCallback() { } // Read not-in-go. - s := []metrics.Sample{{Name: "/sched/goroutines/not-in-go:goroutines"}} - metrics.Read(s) - if n := s[0].Value.Uint64(); n != 0 { - println("expected 0 not-in-go goroutines, found", n) + if !waitNotInGo("after-callbacks:", 0) { os.Exit(2) } println("OK") @@ -202,12 +221,7 @@ func NotInGoMetricCgoCallAndCallback() { } // Read not-in-go before taking the Ps back. - s := []metrics.Sample{{Name: "/sched/goroutines/not-in-go:goroutines"}} - failed := false - metrics.Read(s) - if n := s[0].Value.Uint64(); n != N { - println("pre-STW: expected", N, "not-in-go goroutines, found", n) - } + failed := !waitNotInGo("pre-STW:", N) // Do something that stops the world to take all the Ps back. // @@ -216,9 +230,8 @@ func NotInGoMetricCgoCallAndCallback() { runtime.ReadMemStats(&m) // Read not-in-go. - metrics.Read(s) - if n := s[0].Value.Uint64(); n != N { - println("post-STW: expected", N, "not-in-go goroutines, found", n) + if !waitNotInGo("post-STW:", N) { + failed = true } // Fail if we get a bad reading. diff --git a/test/codegen/bits.go b/test/codegen/bits.go index 80ccd57c005ef9..7d85c4bcd2233b 100644 --- a/test/codegen/bits.go +++ b/test/codegen/bits.go @@ -94,14 +94,14 @@ func bitsCheckVarU64(a, b uint64) (n int) { // amd64:"BTQ" // arm64:"MOVD [$]1," "LSL" "TST" // loong64:"MOVV [$]1," "SLLV R" "AND" "BNE" - // riscv64:"ANDI [$]63," "MOV [$]1," "SLL " "AND " + // riscv64:"MOV [$]1," "SLL " "AND " -"ANDI" if a&(1<<(b&63)) != 0 { return 1 } // amd64:"BTQ" -"BT. [$]0," // arm64:"LSR" "TBZ [$]0," // loong64:"SRLV" "AND [$]1," "BEQ" - // riscv64:"ANDI [$]63," "SRL " "ANDI [$]1," + // riscv64:"SRL " "ANDI [$]1," -"ANDI [$]63," if (b>>(a&63))&1 != 0 { return 1 } @@ -137,7 +137,7 @@ func bitsSetU64(a, b uint64) (n uint64) { // amd64:"BTSQ" // arm64:"MOVD [$]1," "LSL" "ORR" // loong64:"MOVV [$]1," "SLLV" "OR" - // riscv64:"ANDI [$]63," "MOV [$]1," "SLL " "OR " + // riscv64:"MOV [$]1," "SLL " "OR " -"ANDI" n += b | (1 << (a & 63)) // amd64:"BTSQ [$]63," @@ -165,7 +165,7 @@ func bitsClearU64(a, b uint64) (n uint64) { // amd64:"BTRQ" // arm64:"MOVD [$]1," "LSL" "BIC" // loong64:"MOVV [$]1," "SLLV" "ANDN" - // riscv64:"ANDI [$]63," "MOV [$]1," "SLL " "ANDN" + // riscv64:"MOV [$]1," "SLL " "ANDN" -"ANDI" n += b &^ (1 << (a & 63)) // amd64:"BTRQ [$]63," @@ -209,7 +209,7 @@ func bitsFlipU64(a, b uint64) (n uint64) { // amd64:"BTCQ" // arm64:"MOVD [$]1," "LSL" "EOR" // loong64:"MOVV [$]1," "SLLV" "XOR" - // riscv64:"ANDI [$]63," "MOV [$]1," "SLL " "XOR " + // riscv64:"MOV [$]1," "SLL " "XOR " -"ANDI" n += b ^ (1 << (a & 63)) // amd64:"BTCQ [$]63," @@ -319,14 +319,14 @@ func bitsCheckVarU32(a, b uint32) (n int) { // amd64:"BTL" // arm64:"AND [$]31," "MOVD [$]1," "LSL" "TSTW" // loong64:"MOVV [$]1," "SLL R" "AND R" "MOVWU" "BNE" - // riscv64:"ANDI [$]31," "MOV [$]1," "SLLW" "AND " + // riscv64:"MOV [$]1," "SLLW" "AND " -"ANDI" if a&(1<<(b&31)) != 0 { return 1 } // amd64:"BTL" -"BT. [$]0" // arm64:"AND [$]31," "LSR" "TBZ" // loong64:"SRL R" "AND [$]1," "BEQ" - // riscv64:"ANDI [$]31," "SRLW " "ANDI [$]1," + // riscv64:"SRLW " "ANDI [$]1," -"ANDI [$]31," if (b>>(a&31))&1 != 0 { return 1 } @@ -362,7 +362,7 @@ func bitsSetU32(a, b uint32) (n uint32) { // amd64:"BTSL" // arm64:"AND [$]31," "MOVD [$]1," "LSL" "ORR" // loong64:"MOVV [$]1," "SLL " "OR " - // riscv64:"ANDI [$]31," "MOV [$]1," "SLLW " "OR " + // riscv64:"MOV [$]1," "SLLW " "OR " -"ANDI" n += b | (1 << (a & 31)) // amd64:"ORL [$]-2147483648," @@ -390,7 +390,7 @@ func bitsClearU32(a, b uint32) (n uint32) { // amd64:"BTRL" // arm64:"AND [$]31," "MOVD [$]1," "LSL" "BIC" // loong64:"MOVV [$]1," "SLL R" "ANDN" - // riscv64:"ANDI [$]31," "MOV [$]1," "SLLW " "ANDN" + // riscv64:"MOV [$]1," "SLLW " "ANDN" -"ANDI" n += b &^ (1 << (a & 31)) // amd64:"ANDL [$]2147483647," @@ -418,7 +418,7 @@ func bitsFlipU32(a, b uint32) (n uint32) { // amd64:"BTCL" // arm64:"AND [$]31," "MOVD [$]1," "LSL" "EOR" // loong64:"MOVV [$]1," "SLL R" "XOR" - // riscv64:"ANDI [$]31," "MOV [$]1," "SLLW " "XOR " + // riscv64:"MOV [$]1," "SLLW " "XOR " -"ANDI" n += b ^ (1 << (a & 31)) // amd64:"XORL [$]-2147483648," diff --git a/test/fixedbugs/issue80534.go b/test/fixedbugs/issue80534.go new file mode 100644 index 00000000000000..a1fddf607e57c0 --- /dev/null +++ b/test/fixedbugs/issue80534.go @@ -0,0 +1,15 @@ +// compile + +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +var x [3][3]int + +func main() { + for i := range 3 { + x[i][i] = 0 + } +} diff --git a/test/simd.go b/test/simd.go index 05cc24f3a9c2a3..9d44055979190a 100644 --- a/test/simd.go +++ b/test/simd.go @@ -150,10 +150,40 @@ func ternTricky3(x, y, z archsimd.Int32x8) archsimd.Int32x8 { func vpternlogdPanic() { resultsMask := archsimd.Mask64x8{} - for { // ERROR "has features avx[+]avx2[+]avx512" + // The loop header holds only rematerializable SIMD zeros, which emit + // no code, so it claims no features. + for { resultsMask = archsimd.Mask64x8FromBits(0).Or( // ERROR "has features avx[+]avx2[+]avx512" archsimd.Float64x8{}.Less( archsimd.BroadcastFloat64x8(0))).Or(resultsMask) // ERROR "Rewriting.*ternInt" "Skipping rewrite" fmt.Print(resultsMask.And(resultsMask.And(archsimd.Mask64x8{}))) } } + +type notSIMD struct { + v archsimd.Int8x16 + n int +} + +var cond bool + +// A SIMD-typed zero emits no code (it is just a reference to the fixed +// all-zeros register), so it must imply no CPU features: this function +// must stay compilable for machines without AVX and produce no feature +// diagnostics at all. +func zeroOnly(p *notSIMD) { + p.v = archsimd.Int8x16{} + p.n++ +} + +// Only the block executing a real SIMD instruction claims features; +// they do not leak into the unconditionally executed parts of the +// function through the no-code zero or the merge. +func zeroMerge(p *notSIMD, s []int8) { + x := archsimd.Int8x16{} + if cond { + x = archsimd.LoadInt8x16(s) // ERROR "has features avx$" + } + p.v = x + p.n++ +} diff --git a/test/simd_critical.go b/test/simd_critical.go new file mode 100644 index 00000000000000..5f1163f6351418 --- /dev/null +++ b/test/simd_critical.go @@ -0,0 +1,66 @@ +// errorcheck -0 -d=ssa/critical/debug=1 + +//go:build goexperiment.simd && amd64 + +// Copyright 2026 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Test that blocks created by the critical pass to split critical +// edges inherit the CPU features of the edge they sit on, so that +// later consumers (e.g. regalloc-inserted shuffle copies) can use +// feature-dependent instruction encodings there. There are three +// paths through the pass, each exercised below: +// +// 1. a fresh split block for an edge into a block with a single phi +// 2. a fresh split block for an edge into a block with several phis +// (or none) +// 3. a split block reused for several predecessor edges carrying the +// same phi argument, which keeps only the features all of its +// predecessors guarantee + +package foo + +import "simd/archsimd" + +var cond bool + +// Case 1: the merge block has a single phi (x), so the split block for +// the critical edge is created on the single-phi path. It inherits +// avx from its predecessor and successor. +func singlePhi(a, b archsimd.Int64x4) archsimd.Int64x4 { + x := a.Add(b) + if cond { // ERROR "split critical edge" "split-edge block b[0-9]+ has features avx$" + x = x.Add(a) + } + return x +} + +// Case 2: the merge block has two phis (x and y), so the split block +// for the critical edge is created on the no-single-phi path. +func multiPhi(a, b archsimd.Int64x4) (archsimd.Int64x4, archsimd.Int64x4) { + x := a.Add(b) + y := b.Sub(a) + if cond { // ERROR "split critical edge" "split-edge block b[0-9]+ has features avx$" + x = x.Add(a) + y = y.Sub(b) + } + return x, y +} + +// Case 3: the short-circuit && evaluates each operand in its own +// block, and both false edges jump to the single-phi merge block with +// the same phi argument (the zero value of x). The split block is +// created for the edge from the second operand's block, whose features +// are avx+avx2+avx512 (it is dominated by the first operand's block +// and holds the 512-bit ops); when it is reused for the edge from the +// first operand's block, which only guarantees avx, its features must +// drop to the common subset rather than keep avx512. +func reuseIntersect(s []int64, s8 []int64) { + var x archsimd.Int64x4 + if archsimd.LoadInt64x4(s).IsZero() && // ERROR "split critical edge" "reused split-edge block b[0-9]+ has features avx$" "split-edge block b[0-9]+ has features avx$" + archsimd.LoadInt64x8(s8).Equal(archsimd.LoadInt64x8(s8)).ToBits() != 0 { // ERROR "split critical edge" "split-edge block b[0-9]+ has features avx[+]avx2[+]avx512$" + x = archsimd.LoadInt64x4(s) + } + x.Store(s) +}