]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: order nil checks by source position
authorKeith Randall <khr@google.com>
Tue, 27 Nov 2018 22:15:51 +0000 (14:15 -0800)
committerKeith Randall <khr@golang.org>
Wed, 28 Nov 2018 16:49:31 +0000 (16:49 +0000)
There's nothing enforcing ordering between redundant nil checks when
they may occur during the same memory state. Commit to using the
earliest one in source order.

Otherwise the choice of which to remove depends on the ordering of
values in a block (before scheduling). That's unfortunate when trying
to ensure that the compiler doesn't depend on that ordering for
anything.

Update #20178

Change-Id: I2cdd5be10618accd9d91fa07406c90cbd023ffba
Reviewed-on: https://go-review.googlesource.com/c/151517
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
src/cmd/compile/internal/ssa/schedule.go

index e7ad5ac900e2c2a405914b5b5c67e31ba31b3ca5..1f9edb1937d0390a2f79cb104603cb464b76426b 100644 (file)
@@ -4,7 +4,10 @@
 
 package ssa
 
-import "container/heap"
+import (
+       "container/heap"
+       "sort"
+)
 
 const (
        ScorePhi = iota // towards top of block
@@ -447,5 +450,33 @@ func storeOrder(values []*Value, sset *sparseSet, storeNumber []int32) []*Value
                count[s-1]++
        }
 
+       // Order nil checks in source order. We want the first in source order to trigger.
+       // If two are on the same line, we don't really care which happens first.
+       // See issue 18169.
+       if hasNilCheck {
+               start := -1
+               for i, v := range order {
+                       if v.Op == OpNilCheck {
+                               if start == -1 {
+                                       start = i
+                               }
+                       } else {
+                               if start != -1 {
+                                       sort.Sort(bySourcePos(order[start:i]))
+                                       start = -1
+                               }
+                       }
+               }
+               if start != -1 {
+                       sort.Sort(bySourcePos(order[start:]))
+               }
+       }
+
        return order
 }
+
+type bySourcePos []*Value
+
+func (s bySourcePos) Len() int           { return len(s) }
+func (s bySourcePos) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
+func (s bySourcePos) Less(i, j int) bool { return s[i].Pos.Before(s[j].Pos) }