[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657 Richard Biener changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #23 from Richard Biener --- Fixed I guess (and the FRE issue as well).
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657
--- Comment #22 from CVS Commits ---
The master branch has been updated by Jakub Jelinek :
https://gcc.gnu.org/g:866555b170016c49beb869a78cbecdeb07c63135
commit r13-6083-g866555b170016c49beb869a78cbecdeb07c63135
Author: Jakub Jelinek
Date: Thu Feb 16 15:35:05 2023 +0100
tree-ssa-dse: Fix up handling of lhs of internal calls [PR108657]
The r13-1778 PR106378 tree-ssa-dse change didn't just add special support
for IFN_LEN_STORE and IFN_MASK_STORE internal function calls as I believe
was intended, but given that the function was
if (is builtin) { ... }
else if (lhs present and non-SSA_NAME) { ... }
return false;
and it added a new
else if (is internal builtin) { ... }
in between the two, the last if used to be done before on all stmts
with non-SSA_NAME lhs except for calls to builtin functions, but newly
isn't done also for calls to internal functions. In the testcase
the important internal function is .DEFERRED_INIT, which often has
non-SSA_NAME lhs, and the change resulted in them no longer being DSEd,
so a block with nothing in it left but var = .DEFERRED_INIT () and
var = {CLOBBER} was unrolled several times.
The following patch does the lhs handling for all stmts with non-SSA_NAME
lhs
unless initialize_ao_ref_for_dse handled those specially already and
returned (which is the case for various mem* builtins which don't have
such lhs, for some cases of calloc which again is fine,and since r13-1778
also for IFN_LEN_STORE call and some IFN_MASK_STORE calls.
As IFN_MASK_STORE doesn't have a lhs, the break for the !may_def_ok case
doesn't seem to change anything, and because we've handled internal fns
that way in the past, I think it is the right thing to do that again.
That said, if it is inappropriate for some new ifn, I guess it could
be added to the switch and just return false; for it instead of break;.
2023-02-16 Jakub Jelinek
PR tree-optimization/108657
* tree-ssa-dse.cc (initialize_ao_ref_for_dse): If lhs of stmt
exists and is not a SSA_NAME, call ao_ref_init even if the stmt
is a call to internal or builtin function.
* gcc.dg/pr108657.c: New test.
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657 Jakub Jelinek changed: What|Removed |Added Priority|P3 |P1 Keywords|needs-reduction |
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657 --- Comment #21 from Jakub Jelinek --- Created attachment 54472 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54472=edit gcc13-pr108657.patch Untested fix. This one fixes the DSE bug which caused the .DEFERRED_INIT calls not to be DSEd. Though, I wonder if we still don't have a bug in FRE...
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657
--- Comment #20 from Jakub Jelinek ---
Looking at the r13-1778 change, perhaps it wasn't intended that it changed
behavior for all internal functions that return non-SSA_NAME result.
--- gcc/tree-ssa-dse.cc.jj 2023-01-11 10:29:08.651161134 +0100
+++ gcc/tree-ssa-dse.cc 2023-02-15 20:03:33.647684713 +0100
@@ -177,7 +177,7 @@ initialize_ao_ref_for_dse (gimple *stmt,
default:;
}
}
- else if (tree lhs = gimple_get_lhs (stmt))
+ if (tree lhs = gimple_get_lhs (stmt))
{
if (TREE_CODE (lhs) != SSA_NAME)
{
fixes this.
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657
--- Comment #19 from Jakub Jelinek ---
With the deferred stuff (why don't we DCE it for variables which are otherwise
not used?), all looks fine until fre5.
In fre4 we have
k ={v} {CLOBBER(eol)};
c = -1;
c = 1;
foo ();
return 0;
at the end which is reasonable, the outer loop iterates c = 1, c = 0 and c = -1
and then c = 1 is stored, dse4 optimizes it to:
k ={v} {CLOBBER(eol)};
c = 1;
foo ();
return 0;
in split-path, we have in bb2
c = 1;
then in one of the conditionalized bbs
c = 0;
and finally still
[local count: 38651690]:
k ={v} {CLOBBER(eol)};
c = 1;
foo ();
return 0;
at the end, but fre5 keeps those c = 1; and c = 0; early stores and drops the
last one.
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657 --- Comment #18 from Andrew Pinski --- fre5 deletes it: Deleted redundant store c = 1; Value numbering stmt = foo (); Setting value number of .MEM_14 to .MEM_14 (changed) Value numbering stmt = return 0; RPO iteration over 7 blocks visited 7 blocks in total discovering 7 executable blocks iterating 1.0 times, a block was visited max. 1 times RPO tracked 20 values available at 1 locations and 20 lattice elements Removing dead stmt c = 1;
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657
Jakub Jelinek changed:
What|Removed |Added
Ever confirmed|0 |1
Status|UNCONFIRMED |NEW
Last reconfirmed||2023-02-15
--- Comment #17 from Jakub Jelinek ---
In the assembly, the important difference is that with just -O3 c is assigned 1
before calling the first bar, while -O3 -ftrivial-auto-var-init=zero it is
assigned 0. This is visible already in optimized dump, which for the latter
stores first c = 1; and then twice c = 0; among the various DEFERRED_INITs.
Even further reduced:
int c, e, f;
static int *d =
__attribute__((noipa)) void
foo (void)
{
if (c != 1)
__builtin_abort ();
}
int
main ()
{
for (c = 1; c >= 0; c--)
{
e = 0;
for (int j = 0; j <= 2; j++)
{
short k[1];
if (e)
break;
e ^= f;
}
}
*d = 1;
foo ();
}
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657
--- Comment #16 from Jakub Jelinek ---
Slightly more reduced:
int a = -8, c, e, f, g, i;
short b;
static int *d =
int m[256];
unsigned n = ~0U;
void
foo (int b)
{
n = (n >> 8) ^ m[(n ^ b) & 255];
}
void
bar (long x)
{
n = (n >> 8) ^ m[(n ^ x) & 255];
foo (x >> 8);
foo (x >> 16);
}
int
main ()
{
if (__CHAR_BIT__ != 8 || __SIZEOF_SHORT__ != 2 || __SIZEOF_INT__ != 4)
return 0;
for (g = 0; g < 256; g++)
m[g] = g;
b = 53935 & a;
a = -18;
for (c = 1; c >= 0; c--)
{
e = 0;
for (int j = 0; j <= 3; j++)
{
short k[1];
if (e)
break;
e ^= f;
}
}
*d = 8 || 0;
bar (a);
bar (b);
bar (2185655400);
bar (c);
if (n != 0x30)
__builtin_abort ();
}
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657
Jakub Jelinek changed:
What|Removed |Added
CC||jakub at gcc dot gnu.org
--- Comment #15 from Jakub Jelinek ---
Slightly cleaned up; aborts with -O3 -ftrivial-auto-var-init=zero,
doesn't abort with -O0, -O3 or -O0 -ftrivial-auto-var-init=zero.
int m[256];
unsigned n = 4294967295;
void
foo (int b)
{
n = (n >> 8) ^ m[(n ^ b) & 255];
}
void
bar (long x)
{
n = (n >> 8) ^ m[(n ^ x) & 255];
foo (x >> 8);
foo (x >> 16);
foo (x >> 24);
foo (x >> 32);
foo (x >> 40);
foo (x >> 48);
foo (x >> 56);
}
int a = -8, c, e, f, g, i;
short b;
static int *d =
unsigned h;
int
main ()
{
if (__CHAR_BIT__ != 8 || __SIZEOF_SHORT__ != 2 || __SIZEOF_INT__ != 4)
return 0;
for (; g < 256; g++)
{
h = g;
for (i = 8; i; i--)
if (h & 1)
h = (h >> 1) ^ 3988292384;
else
h >>= 1;
m[g] = h;
}
b = 53935 & a;
a = -18;
for (c = 1; c >= 0; c -= 1)
{
e = 0;
for (int j = 0; j <= 3; j++)
{
short k[1];
if (e)
break;
e ^= f;
}
}
*d = 8 || 0;
bar (a);
bar (b);
bar (2185655400);
bar (c);
if (n != 0xbd51857cU)
__builtin_abort ();
}
The only difference between -O3 and -O3 -ftrivial-auto-var-init=zero in gimple
dump
is the addition of
k = .DEFERRED_INIT (2, 2, &"k"[0]);
where k is otherwise unused variable.
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657
--- Comment #14 from David Binderman ---
After five hours of reduction, cvise has:
crc32_tab[256];
unsigned crc32_context = 4294967295;
void crc32_byte(b) {
crc32_context = crc32_context >> 8 ^ crc32_tab[(crc32_context ^ b) & 255];
}
void transparent_crc(long val, char *vname) {
crc32_context = crc32_context >> 8 ^ crc32_tab[(crc32_context ^ val) & 255];
crc32_byte(val >> 8);
crc32_byte(val >> 16);
crc32_byte(val >> 24);
crc32_byte(val >> 32);
crc32_byte(val >> 40);
crc32_byte(val >> 48);
crc32_byte(val >> 56);
printf("...checksum after hashing %s : %lX\n", vname,
crc32_context ^ 4294967295);
}
g_21 = -8;
short g_41;
g_78;
static *g_451 = _78;
g_452;
func_11___trans_tmp_13;
main_i;
unsigned main_crc;
main_j;
void main() {
for (; main_i < 256; main_i++) {
main_crc = main_i;
main_j = 8;
for (; main_j; main_j--)
if (main_crc & 1)
main_crc = main_crc >> 1 ^ 3988292384;
else
main_crc >>= 1;
crc32_tab[main_i] = main_crc;
}
int l_794;
g_41 = 53935 & g_21;
g_21 = -18;
g_78 = 1;
for (; g_78 >= 0; g_78 -= 1) {
g_452 = l_794 = 0;
for (; l_794 <= 3; l_794 += 1) {
short l_895[1];
if (g_452)
break;
g_452 ^= func_11___trans_tmp_13;
}
}
*g_451 = 8 || 0;
transparent_crc(g_21, "");
transparent_crc(g_41, "");
transparent_crc(2185655400, "");
transparent_crc(g_78, "g_78");
}
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657 --- Comment #13 from David Binderman --- I tried adding flag 1 to the run of the two binaries. Here is the bash history: 1003 gcc bug880.c 1004 ./a.out > /tmp/0 1005 ~/gcc/results/bin/gcc -w -O3 -ftrivial-auto-var-init=zero bug880.c -o b.out 1006 ./b.out > /tmp/1 1007 diff /tmp/0 /tmp/1 1008 ./a.out 1 > /tmp/0 1009 ./b.out 1 > /tmp/1 1010 diff /tmp/0 /tmp/1 and got this: 4,12c4,12 < ...checksum after hashing g_78 : 42AE7A83 < ...checksum after hashing g_86 : EA4E5BB < ...checksum after hashing g_87 : 829B71B1 < ...checksum after hashing g_93 : 454CD723 < ...checksum after hashing g_105 : 589C9AA7 < ...checksum after hashing g_111 : D1C885DD < ...checksum after hashing g_132 : A790B35F < ...checksum after hashing g_133 : A9573918 < ...checksum after hashing g_134[i] : 9B3BD0DE --- > ...checksum after hashing g_78 : 8E047A1D > ...checksum after hashing g_86 : A0CC742A > ...checksum after hashing g_87 : 3BE1496 > ...checksum after hashing g_93 : B496D289 > ...checksum after hashing g_105 : F7D808E0 > ...checksum after hashing g_111 : EC796901 > ...checksum after hashing g_132 : BA05A088 > ...checksum after hashing g_133 : 2662148D > ...checksum after hashing g_134[i] : 3530834A So someplace in g_78 looks to be the culprit.
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657 David Binderman changed: What|Removed |Added CC||rguenther at suse dot de --- Comment #12 from David Binderman --- Perhaps Richard would be willing to offer their best advice.
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657 --- Comment #11 from Mikael Pettersson --- Bisected on x86_64-linux-gnu: dc477ffb4aba21e9cf47de22a4df6f2b23849505 is the first bad commit commit dc477ffb4aba21e9cf47de22a4df6f2b23849505 Author: Richard Biener Date: Thu Jul 21 10:13:46 2022 +0200 tree-optimization/106378 - DSE of LEN_STORE and MASK_STORE The following enhances DSE to handle LEN_STORE (optimally) and MASK_STORE (conservatively). PR tree-optimization/106378 * tree-ssa-dse.cc (initialize_ao_ref_for_dse): Handle LEN_STORE, add mode to initialize a may-def and handle MASK_STORE that way. (dse_optimize_stmt): Query may-defs. Handle internal functions LEN_STORE and MASK_STORE similar to how we handle memory builtins but without byte tracking.
[Bug middle-end/108657] [13 Regression] csmith: possible wrong checksum with -O3 and -ftrivial-auto-var-init=zero
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108657 Andrew Pinski changed: What|Removed |Added Summary|csmith: possible wrong |[13 Regression] csmith: |checksum with -O3 and |possible wrong checksum |-ftrivial-auto-var-init=zer |with -O3 and |o |-ftrivial-auto-var-init=zer ||o Target Milestone|--- |13.0
