Hi!
The following testcase is miscompiled, because we emit a CLOBBER in a place
where it shouldn't be emitted.
Before lowering we have:
b_5 = 0;
b.0_6 = b_5;
b.1_1 = (unsigned _BitInt(129)) b.0_6;
...
<retval> = b_5;
The bitint coalescing assigns the same partition/underlying variable
for both b_5 and b.0_6 (possible because there is a copy assignment)
and of course a different one for b.1_1 (and other SSA_NAMEs in between).
This is -O0 so stmts aren't DCEd and aren't propagated that much etc.
It is -O0 so we also don't try to optimize and omit some names from m_names
and handle multiple stmts at once, so the expansion emits essentially
bitint.4 = {};
bitint.4 = bitint.4;
bitint.2 = cast of bitint.4;
bitint.4 = CLOBBER;
...
<retval> = bitint.4;
and the CLOBBER is the problem because bitint.4 is still live afterwards.
We emit the clobbers to improve code generation, but do it only for
(initially) has_single_use SSA_NAMEs (remembered in m_single_use_names)
being used, if they don't have the same partition on the lhs and a few
other conditions.
The problem above is that b.0_6 which is used in the cast has_single_use
and so was in m_single_use_names bitmask and the lhs in that case is
bitint.2, so a different partition. But there is gimple_assign_copy_p
with SSA_NAME rhs1 and the partitioning special cases those and while
b.0_6 is single use, b_5 has multiple uses. I believe this ought to be
a problem solely in the case of such copy stmts and its special case
by the partitioning, if instead of b.0_6 = b_5; there would be
b.0_6 = b_5 + 1; or whatever other stmts that performs or may perform
changes on the value, partitioning couldn't assign the same partition
to b.0_6 and b_5 if b_5 is used later, it couldn't have two different
(or potentially different) values in the same bitint.N var. With
copy that is possible though.
So the following patch fixes it by being more careful when we set
m_single_use_names, don't set it if it is a has_single_use SSA_NAME
but SSA_NAME_DEF_STMT of it is a copy stmt with SSA_NAME rhs1 and that
rhs1 doesn't have single use, or has_single_use but SSA_NAME_DEF_STMT of it
is a copy stmt etc.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Just to make sure it doesn't change code generation too much, I've gathered
statistics how many times
if (m_first
&& m_single_use_names
&& m_vars[p] != m_lhs
&& m_after_stmt
&& bitmap_bit_p (m_single_use_names, SSA_NAME_VERSION (op)))
{
tree clobber = build_clobber (TREE_TYPE (m_vars[p]),
CLOBBER_STORAGE_END);
g = gimple_build_assign (m_vars[p], clobber);
gimple_stmt_iterator gsi = gsi_for_stmt (m_after_stmt);
gsi_insert_after (&gsi, g, GSI_SAME_STMT);
}
emits a clobber on
make check-gcc GCC_TEST_RUN_EXPENSIVE=1
RUNTESTFLAGS="--target_board=unix\{-m64,-m32\} GCC_TEST_RUN_EXPENSIVE=1
dg.exp='*bitint* pr112673.c builtin-stdc-bit-*.c pr112566-2.c pr112511.c
pr116588.c pr116003.c pr113693.c pr113602.c flex-array-counted-by-7.c'
dg-torture.exp='*bitint* pr116480-2.c pr114312.c pr114121.c' dfp.exp=*bitint*
i386.exp='pr118017.c pr117946.c apx-ndd-x32-2a.c'
vect.exp='vect-early-break_99-pr113287.c' tree-ssa.exp=pr113735.c"
and before this patch it was 41010 clobbers and after it is 40968,
so difference is 42 clobbers, 0.1% fewer.
2025-04-16 Jakub Jelinek <ja...@redhat.com>
PR middle-end/119808
* gimple-lower-bitint.cc (gimple_lower_bitint): Don't set
m_single_use_names bits for SSA_NAMEs which have single use but
their SSA_NAME_DEF_STMT is a copy from another SSA_NAME which doesn't
have a single use, or single use which is such a copy etc.
* gcc.dg/bitint-121.c: New test.
--- gcc/gimple-lower-bitint.cc.jj 2025-04-12 13:13:47.543814860 +0200
+++ gcc/gimple-lower-bitint.cc 2025-04-15 21:00:32.779348865 +0200
@@ -6647,10 +6647,28 @@ gimple_lower_bitint (void)
bitmap_set_bit (large_huge.m_names, SSA_NAME_VERSION (s));
if (has_single_use (s))
{
- if (!large_huge.m_single_use_names)
- large_huge.m_single_use_names = BITMAP_ALLOC (NULL);
- bitmap_set_bit (large_huge.m_single_use_names,
- SSA_NAME_VERSION (s));
+ tree s2 = s;
+ /* The coalescing hook special cases SSA_NAME copies.
+ Make sure not to mark in m_single_use_names single
+ use SSA_NAMEs copied from non-single use SSA_NAMEs. */
+ while (gimple_assign_copy_p (SSA_NAME_DEF_STMT (s2)))
+ {
+ s2 = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (s2));
+ if (TREE_CODE (s2) != SSA_NAME)
+ break;
+ if (!has_single_use (s2))
+ {
+ s2 = NULL_TREE;
+ break;
+ }
+ }
+ if (s2)
+ {
+ if (!large_huge.m_single_use_names)
+ large_huge.m_single_use_names = BITMAP_ALLOC (NULL);
+ bitmap_set_bit (large_huge.m_single_use_names,
+ SSA_NAME_VERSION (s));
+ }
}
if (SSA_NAME_VAR (s)
&& ((TREE_CODE (SSA_NAME_VAR (s)) == PARM_DECL
--- gcc/testsuite/gcc.dg/bitint-121.c.jj 2025-04-15 21:03:30.314955543
+0200
+++ gcc/testsuite/gcc.dg/bitint-121.c 2025-04-15 21:03:12.704192945 +0200
@@ -0,0 +1,24 @@
+/* PR middle-end/119808 */
+/* { dg-do run { target { bitint && fstack_protector } } } */
+/* { dg-options "-O0 -ftree-coalesce-vars -fstack-protector-strong" } */
+
+#if __BITINT_MAXWIDTH__ >= 129
+_BitInt(129)
+foo ()
+{
+ _BitInt(129) b = 0;
+ _BitInt(8) a
+ =__builtin_stdc_rotate_right (0x8c82111b5d2d37c57e9ada7213ed95a49uwb, b);
+ return b;
+}
+#endif
+
+int
+main ()
+{
+#if __BITINT_MAXWIDTH__ >= 129
+ _BitInt(129) x = foo ();
+ if (x)
+ __builtin_abort ();
+#endif
+}
Jakub
