Hi!
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;.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
That said, while this patch fixes the regression by allowing DSE of
IFN_DEFERRED_INIT again, I think we probably have some latent bug in FRE
where without this patch it seems to be fre5 that sees one unconditional
c = 1; store, one conditional c = 0; store and in the last bb before return
another c = 1; store and decides that the last store is redundant, which is
not the case, the first two stores are redundant or if they can't be
removed, none of them is. Richard, could you please have a look?
2023-02-15 Jakub Jelinek <ja...@redhat.com>
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.
--- 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)
{
--- gcc/testsuite/gcc.dg/pr108657.c.jj 2023-02-15 20:11:22.038804168 +0100
+++ gcc/testsuite/gcc.dg/pr108657.c 2023-02-15 20:10:37.992451199 +0100
@@ -0,0 +1,31 @@
+/* PR tree-optimization/108657 */
+/* { dg-do run } */
+/* { dg-options "-O3 -ftrivial-auto-var-init=zero" } */
+
+int c, e, f;
+static int *d = &c;
+
+__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 ();
+}
Jakub
