[dev.ssa] cmd/compile: move tuple selectors to generator's block in CSE

CSE may substitute a tuple generator with another one in a different
block. In this case, since we want tuple selectors to stay together
with the tuple generator, copy the selector to the new generator's
block and rewrite its use.

Op.isTupleGenerator and Op.isTupleSelector are introduced to assert
tuple ops. Use it in tighten as well.

Updates #15365.

Change-Id: Ia9e8c734b9cc3bc9fca4a2750041eef9cdfac5a5
Reviewed-on: https://go-review.googlesource.com/24137
Reviewed-by: David Chase <drchase@google.com>

diff --git a/src/cmd/compile/internal/ssa/cse.go b/src/cmd/compile/internal/ssa/cse.go
index ad4e4161591d0ada2ca49b03544bbd306d61e8ac..dcb6eb86ddbed8bca3f28daf1ababc2e131360d3 100644 (file)
--- a/src/cmd/compile/internal/ssa/cse.go
+++ b/src/cmd/compile/internal/ssa/cse.go
@@ -163,6 +163,29 @@ func cse(f *Func) {
                }
        }
 
+       // if we rewrite a tuple generator to a new one in a different block,
+       // copy its selectors to the new generator's block, so tuple generator
+       // and selectors stay together.
+       for _, b := range f.Blocks {
+               for _, v := range b.Values {
+                       if rewrite[v.ID] != nil {
+                               continue
+                       }
+                       if !v.Op.isTupleSelector() {
+                               continue
+                       }
+                       if !v.Args[0].Op.isTupleGenerator() {
+                               f.Fatalf("arg of tuple selector %s is not a tuple: %s", v.String(), v.Args[0].LongString())
+                       }
+                       t := rewrite[v.Args[0].ID]
+                       if t != nil && t.Block != b {
+                               // v.Args[0] is tuple generator, CSE'd into a different block as t, v is left behind
+                               c := v.copyInto(t.Block)
+                               rewrite[v.ID] = c
+                       }
+               }
+       }
+
        rewrites := int64(0)
 
        // Apply substitutions

diff --git a/src/cmd/compile/internal/ssa/op.go b/src/cmd/compile/internal/ssa/op.go
index cadbc7cd7af5c946d9ffe6867e94a064c7f2d9b3..788a8397b3cb2e58a0cff77bc45fdf843a4800fb 100644 (file)
--- a/src/cmd/compile/internal/ssa/op.go
+++ b/src/cmd/compile/internal/ssa/op.go
@@ -124,3 +124,21 @@ func (x ValAndOff) add(off int64) int64 {
        }
        return makeValAndOff(x.Val(), x.Off()+off)
 }
+
+func (op Op) isTupleGenerator() bool {
+       switch op {
+       case OpAdd32carry, OpSub32carry, OpMul32uhilo,
+               OpARMADDS, OpARMSUBS, OpARMMULLU:
+               return true
+       }
+       return false
+}
+
+func (op Op) isTupleSelector() bool {
+       switch op {
+       case OpSelect0, OpSelect1,
+               OpARMLoweredSelect0, OpARMLoweredSelect1, OpARMCarry:
+               return true
+       }
+       return false
+}

diff --git a/src/cmd/compile/internal/ssa/tighten.go b/src/cmd/compile/internal/ssa/tighten.go
index 7f800655b06599049d5367e7ab773150a5f8cf2d..56857b4a265ce9ebdc524cd4c171f374705c6b28 100644 (file)
--- a/src/cmd/compile/internal/ssa/tighten.go
+++ b/src/cmd/compile/internal/ssa/tighten.go
@@ -55,16 +55,19 @@ func tighten(f *Func) {
                        for i := 0; i < len(b.Values); i++ {
                                v := b.Values[i]
                                switch v.Op {
-                               case OpPhi, OpGetClosurePtr, OpConvert, OpArg, OpSelect0, OpSelect1:
+                               case OpPhi, OpGetClosurePtr, OpConvert, OpArg:
                                        // GetClosurePtr & Arg must stay in entry block.
                                        // OpConvert must not float over call sites.
-                                       // Select{0,1} reads a tuple, it must stay with the tuple-generating op.
                                        // TODO do we instead need a dependence edge of some sort for OpConvert?
                                        // Would memory do the trick, or do we need something else that relates
                                        // to safe point operations?
                                        continue
                                default:
                                }
+                               if v.Op.isTupleSelector() {
+                                       // tuple selector must stay with tuple generator
+                                       continue
+                               }
                                if len(v.Args) > 0 && v.Args[len(v.Args)-1].Type.IsMemory() {
                                        // We can't move values which have a memory arg - it might
                                        // make two memory values live across a block boundary.