[Bug middle-end/119808] wrong code with _BitInt() and -ftree-coalesce-vars -fstack-protector-strong
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119808 --- Comment #10 from Andrew Pinski --- (In reply to Jakub Jelinek from comment #9) > Fixed for 14.3 as well, though the #c7 case still needs to be analyzed. I can't reproduce testcase in comment #7.
[Bug middle-end/119808] wrong code with _BitInt() and -ftree-coalesce-vars -fstack-protector-strong
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119808 --- Comment #9 from Jakub Jelinek --- Fixed for 14.3 as well, though the #c7 case still needs to be analyzed.
[Bug middle-end/119808] wrong code with _BitInt() and -ftree-coalesce-vars -fstack-protector-strong
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119808
--- Comment #8 from GCC Commits ---
The releases/gcc-14 branch has been updated by Jakub Jelinek
:
https://gcc.gnu.org/g:40c50659569f9d50db9108a5a9847518a5bcec66
commit r14-11662-g40c50659569f9d50db9108a5a9847518a5bcec66
Author: Jakub Jelinek
Date: Wed Apr 16 09:11:06 2025 +0200
bitintlower: Fix interaction of gimple_assign_copy_p stmts vs.
has_single_use [PR119808]
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;
...
= 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;
...
= 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.
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
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.
(cherry picked from commit 5a48e7732d6aa0aaf12b508fa640125e6c4d14b9)
[Bug middle-end/119808] wrong code with _BitInt() and -ftree-coalesce-vars -fstack-protector-strong
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119808
--- Comment #7 from Zdenek Sojka ---
Created attachment 61132
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=61132&action=edit
testcase failing even with the patch applied
Thank you for the fix; I have another testcase that cvise fails to reduce
further, it has a lot of unused variables and code. The beef of the testcase
is:
short foo0_u16_0;
void foo0 (_BitInt (8) ub8_0, short s16_0, char *ret)
{
ub8_0 *= (foo0_u16_0 % (_BitInt (511)) 396150) & s16_0;
*ret = ub8_0;
}
int
main ()
{
char x;
foo0 (5, 4, &x);
for (unsigned i = 0; i < sizeof (x); i++)
__builtin_printf ("%02x", i[(volatile unsigned char *) &x]);
}
it should print 00, but it prints:
$ x86_64-pc-linux-gnu-gcc -fwrapv -ftrivial-auto-var-init=zero
-ftree-coalesce-vars testcase2.c
$ ./a.out
14
[Bug middle-end/119808] wrong code with _BitInt() and -ftree-coalesce-vars -fstack-protector-strong
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119808 --- Comment #6 from Jakub Jelinek --- Fixed on the trunk so far.
[Bug middle-end/119808] wrong code with _BitInt() and -ftree-coalesce-vars -fstack-protector-strong
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119808
--- Comment #5 from GCC Commits ---
The master branch has been updated by Jakub Jelinek :
https://gcc.gnu.org/g:5a48e7732d6aa0aaf12b508fa640125e6c4d14b9
commit r15-9516-g5a48e7732d6aa0aaf12b508fa640125e6c4d14b9
Author: Jakub Jelinek
Date: Wed Apr 16 09:11:06 2025 +0200
bitintlower: Fix interaction of gimple_assign_copy_p stmts vs.
has_single_use [PR119808]
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;
...
= 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;
...
= 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.
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
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.
[Bug middle-end/119808] wrong code with _BitInt() and -ftree-coalesce-vars -fstack-protector-strong
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119808 Jakub Jelinek changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org --- Comment #4 from Jakub Jelinek --- Created attachment 61126 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=61126&action=edit gcc15-pr119808.patch Untested fix.
[Bug middle-end/119808] wrong code with _BitInt() and -ftree-coalesce-vars -fstack-protector-strong
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119808 --- Comment #3 from Jakub Jelinek --- The clobber is emitted when processing b.1_1 = (unsigned _BitInt(129)) b.0_6; and the rules covering it are 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))) i.e. if it is copying into the same partition or extending etc., m_vars[p] != m_lhs check already guards it, and m_single_use_names is a replacement for has_single_use. b_5 = 0; b.0_6 = b_5; b.1_1 = (unsigned _BitInt(129)) b.0_6; b.0_6 has a single use, the problem was that there was the b.0_6 = b_5; copy first and b_5 doesn't have a single use, yet they share a partition.
[Bug middle-end/119808] wrong code with _BitInt() and -ftree-coalesce-vars -fstack-protector-strong
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119808
--- Comment #2 from Jakub Jelinek ---
The bug is in
bitint.4 ={v} {CLOBBER(eos)};
That shouldn't have been emitted after bitint.4 to bitint.4 copy for the
b.0_6 = b_5;
copy.
I guess for -O1 we should just omit any code for the copy when both have the
same assigned VAR_DECL, for -O0 I think we need to preserve at least something
so that one can put a breakpoint on it. But perhaps we could copy just a
single limb...
[Bug middle-end/119808] wrong code with _BitInt() and -ftree-coalesce-vars -fstack-protector-strong
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=119808
Andrew Pinski changed:
What|Removed |Added
Ever confirmed|0 |1
Last reconfirmed||2025-04-14
Status|UNCONFIRMED |NEW
--- Comment #1 from Andrew Pinski ---
Partition 1: size 24 align 16
bitint.4bitint.3
This is definitely wrong but it is correct based on the IR:
MEM[(unsigned long *)&bitint.4] = 0;
MEM[(unsigned long *)&bitint.4 + 8B] = 0;
MEM[(unsigned long *)&bitint.4 + 16B] = 0;
_100 = MEM[(unsigned long *)&bitint.4];
MEM[(unsigned long *)&bitint.4] = _100;
_101 = MEM[(unsigned long *)&bitint.4 + 8B];
MEM[(unsigned long *)&bitint.4 + 8B] = _101;
_102 = MEM[(unsigned long *)&bitint.4 + 16B];
_103 = () _102;
_104 = (unsigned long) _103;
MEM[(unsigned long *)&bitint.4 + 16B] = _104;
_94 = MEM[(unsigned long *)&bitint.4];
MEM[(unsigned long *)&bitint.2] = _94;
_95 = MEM[(unsigned long *)&bitint.4 + 8B];
MEM[(unsigned long *)&bitint.2 + 8B] = _95;
_96 = MEM[(unsigned long *)&bitint.4 + 16B];
_97 = () _96;
_98 = () _97;
_99 = (unsigned long) _98;
MEM[(unsigned long *)&bitint.2 + 16B] = _99;
bitint.4 ={v} {CLOBBER(eos)};
So the bug is in bitlowering.
b_5 = 0;
b.0_6 = b_5;
b_5 -> bitint.4
b.0_6 -> bitint.4
