trans-mem: Fix ICE caused by expand_assign_tm

My https://gcc.gnu.org/pipermail/gcc-patches/2024-November/668065.html
patch regressed
+FAIL: g++.dg/tm/pr45940-3.C  -std=gnu++11 (internal compiler error: in create_tmp_var, at gimple-expr.cc:484)
+FAIL: g++.dg/tm/pr45940-3.C  -std=gnu++11 (test for excess errors)
+FAIL: g++.dg/tm/pr45940-3.C  -std=gnu++14 (internal compiler error: in create_tmp_var, at gimple-expr.cc:484)
+FAIL: g++.dg/tm/pr45940-3.C  -std=gnu++14 (test for excess errors)
...
+FAIL: g++.dg/tm/pr45940-4.C  -std=gnu++26 (internal compiler error: in create_tmp_var, at gimple-expr.cc:484)
+FAIL: g++.dg/tm/pr45940-4.C  -std=gnu++26 (test for excess errors)
+FAIL: g++.dg/tm/pr45940-4.C  -std=gnu++98 (internal compiler error: in create_tmp_var, at gimple-expr.cc:484)
+FAIL: g++.dg/tm/pr45940-4.C  -std=gnu++98 (test for excess errors)
tests, but it turns out it is a preexisting bug.
If I modify the pr45940-3.C testcase
--- gcc/testsuite/g++.dg/tm/pr45940-3.C 2020-01-12 11:54:37.258400660 +0100
+++ gcc/testsuite/g++.dg/tm/pr45940-3.C 2024-11-08 10:35:11.918390743 +0100
@@ -16,6 +16,7 @@ class sp_counted_base
 {
 protected:
     int use_count_;        // #shared
+    int a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y, z, aa, ab, ac, ad, ae, af;
 public:
     __attribute__((transaction_safe))
     virtual void dispose() = 0; // nothrow
then it ICEs already on vanilla trunk.

The problem is that expand_assign_tm just wants to force it into
TM memcpy argument, if is_gimple_reg (reg), then it creates a temporary,
stores the value there and takes temporary address, otherwise it takes
address of rhs.  That doesn't work if rhs is an empty CONSTRUCTOR with
C++ non-POD type (TREE_ADDRESSABLE type), we ICE trying to create temporary,
because we shouldn't be creating a temporary.
Now before my patch with the CONSTRUCTOR only having a vtable pointer
(64bit) and 32-bit field, we gimplified the zero initialization just
as storing of 0s to the 2 fields, but as we are supposed to also clear
padding bits, we now gimplify it as MEM[...] = {}; to make sure
even the padding bits are cleared.  With the adjusted testcase,
we gimplified it even before as MEM[...] = {}; because it was simply
too large and clearing everything looked beneficial.

The following patch fixes this ICE by using TM memset, it is both
wasteful to force zero constructor into a temporary just to TM memcpy
it into the lhs, and in C++ cases like this invalid.

2024-11-09  Jakub Jelinek  <jakub@redhat.com>

	* trans-mem.cc (expand_assign_tm): Don't take address
	of empty CONSTRUCTOR, instead use BUILT_IN_TM_MEMSET
	to clear lhs in that case.  Formatting fixes.

diff --git a/gcc/trans-mem.cc b/gcc/trans-mem.cc
index 893d5afe49faa54e337349ea7cc26e664c2df6fd..e9139b5fe5345450a611e5e6108bfed9f1c8d3f6 100644 (file)
--- a/gcc/trans-mem.cc
+++ b/gcc/trans-mem.cc
@@ -2442,26 +2442,33 @@ expand_assign_tm (struct tm_region *region, gimple_stmt_iterator *gsi)
          gcall = gimple_build_assign (rtmp, rhs);
          gsi_insert_before (gsi, gcall, GSI_SAME_STMT);
        }
+      else if (TREE_CODE (rhs) == CONSTRUCTOR
+              && CONSTRUCTOR_NELTS (rhs) == 0)
+       {
+         /* Don't take address of an empty CONSTRUCTOR, it might not
+            work for C++ non-POD constructors at all and otherwise
+            would be inefficient.  Use tm memset to clear lhs.  */
+         gcc_assert (!load_p && store_p);
+         rhs_addr = integer_zero_node;
+       }
       else
        rhs_addr = gimplify_addr (gsi, rhs);
 
       // Choose the appropriate memory transfer function.
-      if (load_p && store_p)
-       {
-         // ??? Figure out if there's any possible overlap between
-         // the LHS and the RHS and if not, use MEMCPY.
-         copy_fn = builtin_decl_explicit (BUILT_IN_TM_MEMMOVE);
-       }
+      if (store_p
+         && TREE_CODE (rhs) == CONSTRUCTOR
+         && CONSTRUCTOR_NELTS (rhs) == 0)
+       copy_fn = builtin_decl_explicit (BUILT_IN_TM_MEMSET);
+      else if (load_p && store_p)
+       // ??? Figure out if there's any possible overlap between
+       // the LHS and the RHS and if not, use MEMCPY.
+       copy_fn = builtin_decl_explicit (BUILT_IN_TM_MEMMOVE);
       else if (load_p)
-       {
-         // Note that the store is non-transactional and cannot overlap.
-         copy_fn = builtin_decl_explicit (BUILT_IN_TM_MEMCPY_RTWN);
-       }
+       // Note that the store is non-transactional and cannot overlap.
+       copy_fn = builtin_decl_explicit (BUILT_IN_TM_MEMCPY_RTWN);
       else
-       {
-         // Note that the load is non-transactional and cannot overlap.
-         copy_fn = builtin_decl_explicit (BUILT_IN_TM_MEMCPY_RNWT);
-       }
+       // Note that the load is non-transactional and cannot overlap.
+       copy_fn = builtin_decl_explicit (BUILT_IN_TM_MEMCPY_RNWT);
 
       gcall = gimple_build_call (copy_fn, 3, lhs_addr, rhs_addr,
                                 TYPE_SIZE_UNIT (TREE_TYPE (lhs)));