mirror/gcc
1
0
Fork 0
mirror of https://github.com/gcc-mirror/gcc.git synced 2024-11-21 13:40:47 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

trans-mem: Fix ICE caused by expand_assign_tm

Browse Source 
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.
This commit is contained in:
Jakub Jelinek 2024-11-09 11:42:30 +01:00 committed by Jakub Jelinek
parent 114abf075c
commit fe908af7df
1 changed files with 21 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
gcc/trans-mem.cc
View File

@ -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)));