On Tue, Oct 10, 2023 at 11:59:28AM +0000, Richard Biener wrote:
> > I don't see why the CONSTRUCTOR case couldn't be fine regardless of the
> > vuse. Though, am not really sure when a CONSTRUCTOR would appear, the
> > lhs would need to be an SSA_NAME, so wouldn't for vectors that be a
> > VECTOR_CST instead, etc.? Ah, perhaps a vector with some non-constant
> > element in it.
>
> Yeah, but what should that be interpreted to in terms of object-size?!
The function in question doesn't compute object sizes, just minimum/maximum
number of non-zero bytes in some rhs of a store and whether everything
stored is 0s, or everything non-zeros, or some non-zeros followed by zero.
> I think the only real case we'd see here is the MEM_REF RHS given
> we know we have a register type value. I'll note the function
> totally misses handled_component_p (), it only seems to handle
> *p and 'decl'.
Yeah, maybe we could handle even that at some point.
Though perhaps better first to rewrite it completely, because the recursive
calls where in some cases it means one thing and in another case something
completely different are just bad design (or lack thereof).
> > So maybe pass the vuse down to count_nonzero_bytes_addr and return false
> > in the idx > 0 case in there if gimple_vuse (stmt) != vuse?
>
> I don't know enough of the pass to do better, can you take it from here?
> One of the points is that we need to know the memory context (aka vuse)
> of both the original store and the load we are interpreting. For
> _addr I wasn't sure how we arrive here. As you said, this is a bit
> of spaghetti and I don't want to untangle this any further.
I meant something like below, without getting rid of the -Wshadow stuff
in there my initial attempt didn't work. This passes the new testcase
as well as the testcase you've been touching, but haven't tested it beyond
that yet.
In theory we could even handle some cases with gimple_vuse (stmt) != vuse,
because we save a copy of the strinfo state at the end of basic blocks and
only throw that away after we process all dominator children. But we'd need
to figure out at which bb to look and temporarily switch the vectors.
2023-10-10 Richard Biener <rguent...@suse.de>
Jakub Jelinek <ja...@redhat.com>
PR tree-optimization/111519
* tree-ssa-strlen.cc (strlen_pass::count_nonzero_bytes): Add vuse
argument and pass it through to recursive calls and
count_nonzero_bytes_addr calls. Don't shadow the stmt argument, but
change stmt for gimple_assign_single_p statements for which we don't
immediately punt.
(strlen_pass::count_nonzero_bytes_addr): Add vuse argument and pass
it through to recursive calls and count_nonzero_bytes calls. Don't
use get_strinfo if gimple_vuse (stmt) is different from vuse. Don't
shadow the stmt argument.
* gcc.dg/torture/pr111519.c: New testcase.
--- gcc/tree-ssa-strlen.cc.jj 2023-08-30 11:21:38.539521966 +0200
+++ gcc/tree-ssa-strlen.cc 2023-10-10 15:05:44.731871218 +0200
@@ -281,14 +281,14 @@ public:
gimple *stmt,
unsigned lenrange[3], bool *nulterm,
bool *allnul, bool *allnonnul);
- bool count_nonzero_bytes (tree exp,
+ bool count_nonzero_bytes (tree exp, tree vuse,
gimple *stmt,
unsigned HOST_WIDE_INT offset,
unsigned HOST_WIDE_INT nbytes,
unsigned lenrange[3], bool *nulterm,
bool *allnul, bool *allnonnul,
ssa_name_limit_t &snlim);
- bool count_nonzero_bytes_addr (tree exp,
+ bool count_nonzero_bytes_addr (tree exp, tree vuse,
gimple *stmt,
unsigned HOST_WIDE_INT offset,
unsigned HOST_WIDE_INT nbytes,
@@ -4531,8 +4531,8 @@ nonzero_bytes_for_type (tree type, unsig
}
/* Recursively determine the minimum and maximum number of leading nonzero
- bytes in the representation of EXP and set LENRANGE[0] and LENRANGE[1]
- to each.
+ bytes in the representation of EXP at memory state VUSE and set
+ LENRANGE[0] and LENRANGE[1] to each.
Sets LENRANGE[2] to the total size of the access (which may be less
than LENRANGE[1] when what's being referenced by EXP is a pointer
rather than an array).
@@ -4546,7 +4546,7 @@ nonzero_bytes_for_type (tree type, unsig
Returns true on success and false otherwise. */
bool
-strlen_pass::count_nonzero_bytes (tree exp, gimple *stmt,
+strlen_pass::count_nonzero_bytes (tree exp, tree vuse, gimple *stmt,
unsigned HOST_WIDE_INT offset,
unsigned HOST_WIDE_INT nbytes,
unsigned lenrange[3], bool *nulterm,
@@ -4566,22 +4566,23 @@ strlen_pass::count_nonzero_bytes (tree e
exact value is not known) recurse once to set the range
for an arbitrary constant. */
exp = build_int_cst (type, 1);
- return count_nonzero_bytes (exp, stmt,
+ return count_nonzero_bytes (exp, vuse, stmt,
offset, 1, lenrange,
nulterm, allnul, allnonnul, snlim);
}
- gimple *stmt = SSA_NAME_DEF_STMT (exp);
- if (gimple_assign_single_p (stmt))
+ gimple *g = SSA_NAME_DEF_STMT (exp);
+ if (gimple_assign_single_p (g))
{
- exp = gimple_assign_rhs1 (stmt);
+ exp = gimple_assign_rhs1 (g);
if (!DECL_P (exp)
&& TREE_CODE (exp) != CONSTRUCTOR
&& TREE_CODE (exp) != MEM_REF)
return false;
/* Handle DECLs, CONSTRUCTOR and MEM_REF below. */
+ stmt = g;
}
- else if (gimple_code (stmt) == GIMPLE_PHI)
+ else if (gimple_code (g) == GIMPLE_PHI)
{
/* Avoid processing an SSA_NAME that has already been visited
or if an SSA_NAME limit has been reached. Indicate success
@@ -4590,11 +4591,11 @@ strlen_pass::count_nonzero_bytes (tree e
return res > 0;
/* Determine the minimum and maximum from the PHI arguments. */
- unsigned int n = gimple_phi_num_args (stmt);
+ unsigned int n = gimple_phi_num_args (g);
for (unsigned i = 0; i != n; i++)
{
- tree def = gimple_phi_arg_def (stmt, i);
- if (!count_nonzero_bytes (def, stmt,
+ tree def = gimple_phi_arg_def (g, i);
+ if (!count_nonzero_bytes (def, vuse, g,
offset, nbytes, lenrange, nulterm,
allnul, allnonnul, snlim))
return false;
@@ -4652,7 +4653,7 @@ strlen_pass::count_nonzero_bytes (tree e
return false;
/* Handle MEM_REF = SSA_NAME types of assignments. */
- return count_nonzero_bytes_addr (arg, stmt,
+ return count_nonzero_bytes_addr (arg, vuse, stmt,
offset, nbytes, lenrange, nulterm,
allnul, allnonnul, snlim);
}
@@ -4765,7 +4766,7 @@ strlen_pass::count_nonzero_bytes (tree e
bytes that are pointed to by EXP, which should be a pointer. */
bool
-strlen_pass::count_nonzero_bytes_addr (tree exp, gimple *stmt,
+strlen_pass::count_nonzero_bytes_addr (tree exp, tree vuse, gimple *stmt,
unsigned HOST_WIDE_INT offset,
unsigned HOST_WIDE_INT nbytes,
unsigned lenrange[3], bool *nulterm,
@@ -4775,6 +4776,14 @@ strlen_pass::count_nonzero_bytes_addr (t
int idx = get_stridx (exp, stmt);
if (idx > 0)
{
+ /* get_strinfo reflects string lengths before the current statement,
+ where the current statement is the outermost count_nonzero_bytes
+ stmt. If there are any stores in between stmt and that
+ current statement, the string length information might describe
+ something significantly different. */
+ if (gimple_vuse (stmt) != vuse)
+ return false;
+
strinfo *si = get_strinfo (idx);
if (!si)
return false;
@@ -4835,14 +4844,14 @@ strlen_pass::count_nonzero_bytes_addr (t
}
if (TREE_CODE (exp) == ADDR_EXPR)
- return count_nonzero_bytes (TREE_OPERAND (exp, 0), stmt,
+ return count_nonzero_bytes (TREE_OPERAND (exp, 0), vuse, stmt,
offset, nbytes,
lenrange, nulterm, allnul, allnonnul, snlim);
if (TREE_CODE (exp) == SSA_NAME)
{
- gimple *stmt = SSA_NAME_DEF_STMT (exp);
- if (gimple_code (stmt) == GIMPLE_PHI)
+ gimple *g = SSA_NAME_DEF_STMT (exp);
+ if (gimple_code (g) == GIMPLE_PHI)
{
/* Avoid processing an SSA_NAME that has already been visited
or if an SSA_NAME limit has been reached. Indicate success
@@ -4851,11 +4860,11 @@ strlen_pass::count_nonzero_bytes_addr (t
return res > 0;
/* Determine the minimum and maximum from the PHI arguments. */
- unsigned int n = gimple_phi_num_args (stmt);
+ unsigned int n = gimple_phi_num_args (g);
for (unsigned i = 0; i != n; i++)
{
- tree def = gimple_phi_arg_def (stmt, i);
- if (!count_nonzero_bytes_addr (def, stmt,
+ tree def = gimple_phi_arg_def (g, i);
+ if (!count_nonzero_bytes_addr (def, vuse, g,
offset, nbytes, lenrange,
nulterm, allnul, allnonnul,
snlim))
@@ -4903,7 +4912,7 @@ strlen_pass::count_nonzero_bytes (tree e
ssa_name_limit_t snlim;
tree expr = expr_or_type;
- return count_nonzero_bytes (expr, stmt,
+ return count_nonzero_bytes (expr, gimple_vuse (stmt), stmt,
0, 0, lenrange, nulterm, allnul, allnonnul,
snlim);
}
--- gcc/testsuite/gcc.dg/torture/pr111519.c.jj 2023-10-10 14:36:22.195889648
+0200
+++ gcc/testsuite/gcc.dg/torture/pr111519.c 2023-10-10 14:36:22.195889648
+0200
@@ -0,0 +1,48 @@
+/* PR tree-optimization/111519 */
+/* { dg-do run } */
+
+int a, o;
+char b, f, i;
+long c;
+static signed char d;
+static char g;
+unsigned *h;
+signed char *e = &f;
+static signed char **j = &e;
+static long k[2];
+unsigned **l = &h;
+short m;
+volatile int z;
+
+__attribute__((noipa)) void
+foo (char *p)
+{
+ (void) p;
+}
+
+int
+main ()
+{
+ int p = z;
+ signed char *n = &d;
+ *n = 0;
+ while (c)
+ for (; i; i--)
+ ;
+ for (g = 0; g <= 1; g++)
+ {
+ *n = **j;
+ k[g] = 0 != &m;
+ *e = l && k[0];
+ }
+ if (p)
+ foo (&b);
+ for (; o < 4; o++)
+ {
+ a = d;
+ if (p)
+ foo (&b);
+ }
+ if (a != 1)
+ __builtin_abort ();
+}
Jakub
