]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: simplify uninterruptable range check for write barriers
authorKeith Randall <khr@golang.org>
Tue, 30 May 2023 17:19:50 +0000 (10:19 -0700)
committerKeith Randall <khr@golang.org>
Wed, 26 Jul 2023 17:18:31 +0000 (17:18 +0000)
Make the load detection a bit clearer and more precise. In particular,
for architectures which have to materialize the address using a
separate instruction, we were using the address materialization
instruction, not the load itself.

Also apply the marking a bit less. We don't need to mark the load itself,
only the instructions after the load. And we don't need to mark the WBend
itself, only the instructions before it.

Change-Id: Ie367a8023b003d5317b752d873bb385f931bb30e
Reviewed-on: https://go-review.googlesource.com/c/go/+/499395
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
src/cmd/compile/internal/liveness/plive.go

index 169467e6f5a333944fada0ea12626492d15b98be..2d05ed1a4a49013bbcf7c707e788567d0543862f 100644 (file)
@@ -489,8 +489,6 @@ func (lv *liveness) markUnsafePoints() {
                        //    m2 = store operation ... m1
                        //    m3 = store operation ... m2
                        //    m4 = WBend m3
-                       //
-                       // (For now m2 and m3 won't be present.)
 
                        // Find first memory op in the block, which should be a Phi.
                        m := v
@@ -535,39 +533,36 @@ func (lv *liveness) markUnsafePoints() {
                        var load *ssa.Value
                        v := decisionBlock.Controls[0]
                        for {
-                               if sym, ok := v.Aux.(*obj.LSym); ok && sym == ir.Syms.WriteBarrier {
-                                       load = v
-                                       break
-                               }
-                               switch v.Op {
-                               case ssa.Op386TESTL:
-                                       // 386 lowers Neq32 to (TESTL cond cond),
-                                       if v.Args[0] == v.Args[1] {
-                                               v = v.Args[0]
-                                               continue
+                               if v.MemoryArg() != nil {
+                                       // Single instruction to load (and maybe compare) the write barrier flag.
+                                       if sym, ok := v.Aux.(*obj.LSym); ok && sym == ir.Syms.WriteBarrier {
+                                               load = v
+                                               break
                                        }
-                               case ssa.Op386MOVLload, ssa.OpARM64MOVWUload, ssa.OpMIPS64MOVWUload, ssa.OpPPC64MOVWZload, ssa.OpWasmI64Load32U:
-                                       // Args[0] is the address of the write
-                                       // barrier control. Ignore Args[1],
-                                       // which is the mem operand.
-                                       // TODO: Just ignore mem operands?
-                                       v = v.Args[0]
-                                       continue
+                                       // Some architectures have to materialize the address separate from
+                                       // the load.
+                                       if sym, ok := v.Args[0].Aux.(*obj.LSym); ok && sym == ir.Syms.WriteBarrier {
+                                               load = v
+                                               break
+                                       }
+                                       v.Fatalf("load of write barrier flag not from correct global: %s", v.LongString())
                                }
                                // Common case: just flow backwards.
-                               if len(v.Args) != 1 {
-                                       v.Fatalf("write barrier control value has more than one argument: %s", v.LongString())
+                               if len(v.Args) == 1 || len(v.Args) == 2 && v.Args[0] == v.Args[1] {
+                                       // Note: 386 lowers Neq32 to (TESTL cond cond),
+                                       v = v.Args[0]
+                                       continue
                                }
-                               v = v.Args[0]
+                               v.Fatalf("write barrier control value has more than one argument: %s", v.LongString())
                        }
 
                        // Mark everything after the load unsafe.
                        found := false
                        for _, v := range decisionBlock.Values {
-                               found = found || v == load
                                if found {
                                        lv.unsafePoints.Set(int32(v.ID))
                                }
+                               found = found || v == load
                        }
 
                        // Mark the write barrier on/off blocks as unsafe.
@@ -583,10 +578,10 @@ func (lv *liveness) markUnsafePoints() {
 
                        // Mark from the join point up to the WBend as unsafe.
                        for _, v := range b.Values {
-                               lv.unsafePoints.Set(int32(v.ID))
                                if v.Op == ssa.OpWBend {
                                        break
                                }
+                               lv.unsafePoints.Set(int32(v.ID))
                        }
                }
        }