cmd/compile/internal/ssa: handle more cases in fuse pass

author eric fang <eric.fang@arm.com>

Thu, 11 Jun 2020 09:35:24 +0000 (09:35 +0000)

committer eric fang <eric.fang@arm.com>

Tue, 16 Mar 2021 02:25:37 +0000 (02:25 +0000)
author eric fang <eric.fang@arm.com>
Thu, 11 Jun 2020 09:35:24 +0000 (09:35 +0000)
committer eric fang <eric.fang@arm.com>
Tue, 16 Mar 2021 02:25:37 +0000 (02:25 +0000)
diff --git a/src/cmd/compile/internal/ssa/fuse.go b/src/cmd/compile/internal/ssa/fuse.go

index c51461cbfffedd375ed7a8f07761d1a9d8e9f2bb..236d5bbc55e98680e4707c2ca17f7fdfc7fb650e 100644 (file)
--- a/src/cmd/compile/internal/ssa/fuse.go
+++ b/src/cmd/compile/internal/ssa/fuse.go
@@ -51,11 +51,11 @@ func fuse(f *Func, typ fuseType) {
  
  // fuseBlockIf handles the following cases where s0 and s1 are empty blocks.
  //
-//   b        b        b      b
-//  / \      | \      / |    | |
-// s0  s1    |  s1   s0 |    | |
-//  \ /      | /      \ |    | |
-//   ss      ss        ss     ss
+//       b        b           b       b
+//    \ / \ /    | \  /    \ / |     | |
+//     s0  s1    |  s1      s0 |     | |
+//      \ /      | /         \ |     | |
+//       ss      ss           ss      ss
  //
  // If all Phi ops in ss have identical variables for slots corresponding to
  // s0, s1 and b then the branch can be dropped.
@@ -69,11 +69,11 @@ func fuseBlockIf(b *Block) bool {
         if b.Kind != BlockIf {
                 return false
         }
-
+       // It doesn't matter how much Preds does s0 or s1 have.
         var ss0, ss1 *Block
         s0 := b.Succs[0].b
         i0 := b.Succs[0].i
-       if s0.Kind != BlockPlain || len(s0.Preds) != 1 || !isEmpty(s0) {
+       if s0.Kind != BlockPlain || !isEmpty(s0) {
                 s0, ss0 = b, s0
         } else {
                 ss0 = s0.Succs[0].b
@@ -81,15 +81,25 @@ func fuseBlockIf(b *Block) bool {
         }
         s1 := b.Succs[1].b
         i1 := b.Succs[1].i
-       if s1.Kind != BlockPlain || len(s1.Preds) != 1 || !isEmpty(s1) {
+       if s1.Kind != BlockPlain || !isEmpty(s1) {
                 s1, ss1 = b, s1
         } else {
                 ss1 = s1.Succs[0].b
                 i1 = s1.Succs[0].i
         }
-
         if ss0 != ss1 {
-               return false
+               if s0.Kind == BlockPlain && isEmpty(s0) && s1.Kind == BlockPlain && isEmpty(s1) {
+                       // Two special cases where both s0, s1 and ss are empty blocks.
+                       if s0 == ss1 {
+                               s0, ss0 = b, ss1
+                       } else if ss0 == s1 {
+                               s1, ss1 = b, ss0
+                       } else {
+                               return false
+                       }
+               } else {
+                       return false
+               }
         }
         ss := ss0
  
@@ -102,48 +112,45 @@ func fuseBlockIf(b *Block) bool {
                 }
         }
  
-       // Now we have two of following b->ss, b->s0->ss and b->s1->ss,
-       // with s0 and s1 empty if exist.
-       // We can replace it with b->ss without if all OpPhis in ss
-       // have identical predecessors (verified above).
-       // No critical edge is introduced because b will have one successor.
-       if s0 != b && s1 != b {
-               // Replace edge b->s0->ss with b->ss.
-               // We need to keep a slot for Phis corresponding to b.
-               b.Succs[0] = Edge{ss, i0}
-               ss.Preds[i0] = Edge{b, 0}
-               b.removeEdge(1)
-               s1.removeEdge(0)
-       } else if s0 != b {
-               b.removeEdge(0)
+       // We do not need to redirect the Preds of s0 and s1 to ss,
+       // the following optimization will do this.
+       b.removeEdge(0)
+       if s0 != b && len(s0.Preds) == 0 {
                 s0.removeEdge(0)
-       } else if s1 != b {
-               b.removeEdge(1)
-               s1.removeEdge(0)
-       } else {
-               b.removeEdge(1)
+               // Move any (dead) values in s0 to b,
+               // where they will be eliminated by the next deadcode pass.
+               for _, v := range s0.Values {
+                       v.Block = b
+               }
+               b.Values = append(b.Values, s0.Values...)
+               // Clear s0.
+               s0.Kind = BlockInvalid
+               s0.Values = nil
+               s0.Succs = nil
+               s0.Preds = nil
         }
+
         b.Kind = BlockPlain
         b.Likely = BranchUnknown
         b.ResetControls()
-
-       // Trash the empty blocks s0 and s1.
-       blocks := [...]*Block{s0, s1}
-       for _, s := range &blocks {
-               if s == b {
-                       continue
+       // The values in b may be dead codes, and clearing them in time may
+       // obtain new optimization opportunities.
+       // First put dead values that can be deleted into a slice walkValues.
+       // Then put their arguments in walkValues before resetting the dead values
+       // in walkValues, because the arguments may also become dead values.
+       walkValues := []*Value{}
+       for _, v := range b.Values {
+               if v.Uses == 0 && v.removeable() {
+                       walkValues = append(walkValues, v)
                 }
-               // Move any (dead) values in s0 or s1 to b,
-               // where they will be eliminated by the next deadcode pass.
-               for _, v := range s.Values {
-                       v.Block = b
+       }
+       for len(walkValues) != 0 {
+               v := walkValues[len(walkValues)-1]
+               walkValues = walkValues[:len(walkValues)-1]
+               if v.Uses == 0 && v.removeable() {
+                       walkValues = append(walkValues, v.Args...)
+                       v.reset(OpInvalid)
                 }
-               b.Values = append(b.Values, s.Values...)
-               // Clear s.
-               s.Kind = BlockInvalid
-               s.Values = nil
-               s.Succs = nil
-               s.Preds = nil
         }
         return true
  }
diff --git a/src/cmd/compile/internal/ssa/fuse_test.go b/src/cmd/compile/internal/ssa/fuse_test.go

index 15190997f2262f779e2d8bd818e358745c10d637..27a14b17819c5d6c1626a4c242b36cb2f861ce0b 100644 (file)
--- a/src/cmd/compile/internal/ssa/fuse_test.go
+++ b/src/cmd/compile/internal/ssa/fuse_test.go
@@ -104,6 +104,18 @@ func TestFuseHandlesPhis(t *testing.T) {
  
  func TestFuseEliminatesEmptyBlocks(t *testing.T) {
         c := testConfig(t)
+       // Case 1, plain type empty blocks z0 ~ z3 will be eliminated.
+       //     entry
+       //       |
+       //      z0
+       //       |
+       //      z1
+       //       |
+       //      z2
+       //       |
+       //      z3
+       //       |
+       //     exit
         fun := c.Fun("entry",
                 Bloc("entry",
                         Valu("mem", OpInitMem, types.TypeMem, 0, nil),
@@ -126,16 +138,77 @@ func TestFuseEliminatesEmptyBlocks(t *testing.T) {
  
         for k, b := range fun.blocks {
                 if k[:1] == "z" && b.Kind != BlockInvalid {
-                       t.Errorf("%s was not eliminated, but should have", k)
+                       t.Errorf("case1 %s was not eliminated, but should have", k)
+               }
+       }
+
+       // Case 2, empty blocks with If branch, z0 and z1 will be eliminated.
+       //     entry
+       //     /  \
+       //    z0  z1
+       //     \  /
+       //     exit
+       fun = c.Fun("entry",
+               Bloc("entry",
+                       Valu("mem", OpInitMem, types.TypeMem, 0, nil),
+                       Valu("c", OpArg, c.config.Types.Bool, 0, nil),
+                       If("c", "z0", "z1")),
+               Bloc("z0",
+                       Goto("exit")),
+               Bloc("z1",
+                       Goto("exit")),
+               Bloc("exit",
+                       Exit("mem"),
+               ))
+
+       CheckFunc(fun.f)
+       fuseLate(fun.f)
+
+       for k, b := range fun.blocks {
+               if k[:1] == "z" && b.Kind != BlockInvalid {
+                       t.Errorf("case2 %s was not eliminated, but should have", k)
+               }
+       }
+
+       // Case 3, empty blocks with multiple predecessors, z0 and z1 will be eliminated.
+       //     entry
+       //      |  \
+       //      |  b0
+       //      | /  \
+       //      z0   z1
+       //       \   /
+       //       exit
+       fun = c.Fun("entry",
+               Bloc("entry",
+                       Valu("mem", OpInitMem, types.TypeMem, 0, nil),
+                       Valu("c1", OpArg, c.config.Types.Bool, 0, nil),
+                       If("c1", "b0", "z0")),
+               Bloc("b0",
+                       Valu("c2", OpArg, c.config.Types.Bool, 0, nil),
+                       If("c2", "z1", "z0")),
+               Bloc("z0",
+                       Goto("exit")),
+               Bloc("z1",
+                       Goto("exit")),
+               Bloc("exit",
+                       Exit("mem"),
+               ))
+
+       CheckFunc(fun.f)
+       fuseLate(fun.f)
+
+       for k, b := range fun.blocks {
+               if k[:1] == "z" && b.Kind != BlockInvalid {
+                       t.Errorf("case3 %s was not eliminated, but should have", k)
                 }
         }
  }
  
  func TestFuseSideEffects(t *testing.T) {
-       // Test that we don't fuse branches that have side effects but
+       c := testConfig(t)
+       // Case1, test that we don't fuse branches that have side effects but
         // have no use (e.g. followed by infinite loop).
         // See issue #36005.
-       c := testConfig(t)
         fun := c.Fun("entry",
                 Bloc("entry",
                         Valu("mem", OpInitMem, types.TypeMem, 0, nil),
@@ -163,6 +236,31 @@ func TestFuseSideEffects(t *testing.T) {
                         t.Errorf("else is eliminated, but should not")
                 }
         }
+
+       // Case2, z0 contains a value that has side effect, z0 shouldn't be eliminated.
+       //     entry
+       //      | \
+       //      |  z0
+       //      | /
+       //     exit
+       fun = c.Fun("entry",
+               Bloc("entry",
+                       Valu("mem", OpInitMem, types.TypeMem, 0, nil),
+                       Valu("c1", OpArg, c.config.Types.Bool, 0, nil),
+                       Valu("p", OpArg, c.config.Types.IntPtr, 0, nil),
+                       If("c1", "z0", "exit")),
+               Bloc("z0",
+                       Valu("nilcheck", OpNilCheck, types.TypeVoid, 0, nil, "p", "mem"),
+                       Goto("exit")),
+               Bloc("exit",
+                       Exit("mem"),
+               ))
+       CheckFunc(fun.f)
+       fuseLate(fun.f)
+       z0, ok := fun.blocks["z0"]
+       if !ok || z0.Kind == BlockInvalid {
+               t.Errorf("case2 z0 is eliminated, but should not")
+       }
  }
  
  func BenchmarkFuse(b *testing.B) {
author	eric fang <eric.fang@arm.com>
	Thu, 11 Jun 2020 09:35:24 +0000 (09:35 +0000)
committer	eric fang <eric.fang@arm.com>
	Tue, 16 Mar 2021 02:25:37 +0000 (02:25 +0000)
src/cmd/compile/internal/ssa/fuse.go		patch \| blob \| history
src/cmd/compile/internal/ssa/fuse_test.go		patch \| blob \| history