On 10/14/20 4:31 PM, Richard Biener wrote:
On Wed, Oct 14, 2020 at 4:19 PM Aldy Hernandez <al...@redhat.com> wrote:
On 10/14/20 9:43 AM, Richard Biener wrote:
On Tue, Oct 13, 2020 at 6:12 PM Aldy Hernandez <al...@redhat.com> wrote:
On 10/13/20 6:02 PM, Richard Biener wrote:
On October 13, 2020 5:17:48 PM GMT+02:00, Aldy Hernandez via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
[Neither Andrew nor I are familiar with the SCEV code. We treat it as
a
black box :). So we could use a SCEV expert here.]
In bounds_of_var_in_loop, evolution_part_in_loop_num is returning NULL:
step = evolution_part_in_loop_num (chrec, loop->num);
(*)
It means that Var doesn't vary in the loop.
That is, chrec isn't a polynomial chrec.
That's what I thought, but it is:
(gdb) p chrec
$6 = <polynomial_chrec 0x7ffff0e2a820>
(gdb) dd chrec
{0, +, 1}_2
evolution_part_in_loop_num() is returning NULL deep in
chrec_component_in_loop_num():
default:
=> if (right)
return NULL_TREE;
else
return chrec;
Do you have any suggestions?
I can only guess (w/o a testcase) that loop->num at (*) is not 2 and thus that
chrec does not evolve in the loop we're asking. But this doesn't make much
sense with the constraints we are calling this function (a loop header PHI
with loop == the loop and stmt a loop header PHI and var the PHIs lhs).
OK, so looking at the testcase you're doing
492 class loop *l = loop_containing_stmt (phi);
493 if (l)
494 {
495 range_of_ssa_name_with_loop_info (loop_range,
phi_def, l, phi);
but 'l' isn't a loop, it's the loop tree root. Change to
if (l && loop_outer (l))
Woah. I did not expect that.
A quick peek shows that all users of bounds_of_var_in_loop (through
adjust_range_with_scev) predicate the call with:
&& l->header == gimple_bb (phi))
If this check is similar to the loop_outer(l) you suggest, could we
perhaps push this check (and/or the loop_outer one) into
bounds_of_var_in_loop itself and remove it from all the callers? It
seems cleaner to have this check in one place, than in three different
places. That is, unless the l->header check is altogether a different
thing than loop_outer(l).
The code also works for stmts other than PHIs (or stmts in other blocks
than the loop header), so IMHO is not appropriate for bounds_of_var_in_loop.
Ahhh. Makes perfect sense.
The patch below passes tests. I've pushed it.
Thanks.
Aldy
gcc/ChangeLog:
PR tree-optimization/97396
* gimple-range.cc (gimple_ranger::range_of_phi): Do not call
range_of_ssa_name_with_loop_info with the loop tree root.
gcc/testsuite/ChangeLog:
* gcc.dg/pr97396.c: New test.
---
gcc/gimple-range.cc | 2 +-
gcc/testsuite/gcc.dg/pr97396.c | 23 +++++++++++++++++++++++
2 files changed, 24 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/pr97396.c
diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc
index 2ca86ed0e4c..999d631c5ee 100644
--- a/gcc/gimple-range.cc
+++ b/gcc/gimple-range.cc
@@ -490,7 +490,7 @@ gimple_ranger::range_of_phi (irange &r, gphi *phi)
{
value_range loop_range;
class loop *l = loop_containing_stmt (phi);
- if (l)
+ if (l && loop_outer (l))
{
range_of_ssa_name_with_loop_info (loop_range, phi_def, l, phi);
if (!loop_range.varying_p ())
diff --git a/gcc/testsuite/gcc.dg/pr97396.c b/gcc/testsuite/gcc.dg/pr97396.c
new file mode 100644
index 00000000000..d992c11f238
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr97396.c
@@ -0,0 +1,23 @@
+// { dg-do compile }
+// { dg-options "-O1 -ftree-vrp" }
+// { dg-additional-options "-m32" { target { i?86-*-* x86_64-*-* } } }
+
+unsigned int
+po (char *os, unsigned int al)
+{
+ for (;;)
+ {
+ int qx = 0;
+
+ while (al < 1)
+ {
+ char *cw;
+
+ cw = os + qx;
+ if (cw)
+ return al + qx;
+
+ qx += sizeof *cw;
+ }
+ }
+}
--
2.26.2
