[PATCH]middle-end slp: fix accidental resource re-use of slp_tree (PR99220)

[Tamar Christina]

Hi Richi,

The attached testcase shows a bug where two nodes end up with the same pointer.
During the loop that analyzes all the instances
in optimize_load_redistribution_1 we do

      if (value)
        {
          SLP_TREE_REF_COUNT (value)++;
          SLP_TREE_CHILDREN (root)[i] = value;
          vect_free_slp_tree (node);
        }

when doing a replacement.  When this is done and the refcount for the node
reaches 0, the node is removed, which allows the libc to return the pointer
again in the next call to new, which it does..

First instance

note:   node 0x5325f48 (max_nunits=1, refcnt=2)
note:   op: VEC_PERM_EXPR
note:           { }
note:           lane permutation { 0[0] 1[1] 0[2] 1[3] }
note:           children 0x5325db0 0x5325200

Second instance

note:   node 0x5325f48 (max_nunits=1, refcnt=1)
note:   op: VEC_PERM_EXPR
note:           { }
note:           lane permutation { 0[0] 1[1] }
note:           children 0x53255b8 0x5325530

This will end up with the illegal construction of

note:   node 0x53258e8 (max_nunits=2, refcnt=2)
note:   op template: slp_patt_57 = .COMPLEX_MUL (_16, _16);
note:           stmt 0 _16 = _14 - _15;
note:           stmt 1 _23 = _17 + _22;
note:           children 0x53257d8 0x5325d28
note:   node 0x53257d8 (max_nunits=2, refcnt=3)
note:   op template: l$b_4 = MEM[(const struct a &)_3].b;
note:           stmt 0 l$b_4 = MEM[(const struct a &)_3].b;
note:           stmt 1 l$c_5 = MEM[(const struct a &)_3].c;
note:           load permutation { 0 1 }
note:   node 0x5325d28 (max_nunits=2, refcnt=8)
note:   op template: l$b_4 = MEM[(const struct a &)_3].b;
note:           stmt 0 l$b_4 = MEM[(const struct a &)_3].b;
note:           stmt 1 l$c_5 = MEM[(const struct a &)_3].c;
note:           stmt 2 l$b_4 = MEM[(const struct a &)_3].b;
note:           stmt 3 l$c_5 = MEM[(const struct a &)_3].c;
note:           load permutation { 0 1 0 1 }

To prevent this my initial thought was to add the temporary VEC_PERM_EXPR nodes
to the bst_map cache and increase their refcnt one more.  However since bst_map
is gated on scalar statements and these nodes have none we can't do that.

Instead I realized that load_map is really only a visited list at the top level.
So instead of returning the reference, we should return NULL.

What this means is that it will no replacement was found at that level.  This is
fine since these VEC_PERM_EXPR are single use.  So while the any other node is
an indication to use the cache, VEC_PERM_EXPR are an indication to avoid it.

Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.

Ok for master?

Thanks,
Tamar

gcc/ChangeLog:

	PR tree-optimization/99220
	* tree-vect-slp.c (optimize_load_redistribution_1): Don't use
	VEC_PERM_EXPR in cache.

gcc/testsuite/ChangeLog:

	PR tree-optimization/99220
	* g++.dg/vect/pr99220.cc: New test.

--- inline copy of patch -- 
diff --git a/gcc/testsuite/g++.dg/vect/pr99220.cc b/gcc/testsuite/g++.dg/vect/pr99220.cc
new file mode 100755
index 0000000000000000000000000000000000000000..ff3058832b742414202a8ada0a9dafc72c9a54aa
--- /dev/null
+++ b/gcc/testsuite/g++.dg/vect/pr99220.cc
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-w -O3 -march=armv8.3-a" { target { aarch64*-*-* } } } */
+
+class a {
+  float b;
+  float c;
+
+public:
+  a(float d, float e) : b(d), c(e) {}
+  a operator+(a d) { return a(b + d.b, c + d.c); }
+  a operator-(a d) { return a(b - d.b, c - d.c); }
+  a operator*(a d) { return a(b * b - c * c, b * c + c * d.b); }
+};
+long f;
+a *g;
+class {
+  a *h;
+  long i;
+  a *j;
+
+public:
+  void k() {
+    a l = h[0], m = g[i], n = l * g[1], o = l * j[8];
+    g[i] = m + n;
+    g[i + 1] = m - n;
+    j[f] = o;
+  }
+} p;
+main() { p.k(); }
diff --git a/gcc/tree-vect-slp.c b/gcc/tree-vect-slp.c
index 605873714a5cafaaf822f61f1f769f96b3876694..e631463be8fc5b2de355e674a9c96665beb9516c 100644
--- a/gcc/tree-vect-slp.c
+++ b/gcc/tree-vect-slp.c
@@ -2292,7 +2292,12 @@ optimize_load_redistribution_1 (scalar_stmts_to_slp_tree_map_t *bst_map,
 				slp_tree root)
 {
   if (slp_tree *leader = load_map->get (root))
-    return *leader;
+    {
+      if (SLP_TREE_CODE (root) == VEC_PERM_EXPR)
+	return NULL;
+      else
+	return *leader;
+    }
 
   load_map->put (root, NULL);
 


-- 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: rb14228.patch
Type: text/x-diff
Size: 1403 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210223/09d4ff77/attachment.bin>