On Sat, Nov 11, 2023 at 9:36 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> The following testcase ICEs when dumping details.
> When m_ssa_ranges vector is created, it is safe_grow_cleared (num_ssa_names),
> but when when some new SSA_NAME is added, we strangely grow it to
> num_ssa_names + 1 instead and later on the 3 argument dump method
> iterates from 1 to m_ssa_ranges.length () - 1 and uses ssa_name (x)
> on each; but because set_bb_range grew it one too much, ssa_name
> (m_ssa_ranges.length () - 1) might be after the end of the ssanames
> vector and ICE.
>
> The fix grows the vector consistently only to num_ssa_names,
> doesn't waste time checking m_ssa_ranges[0] because there is no
> ssa_names (0), it is always NULL, before using ssa_name (x) checks
> if we'll need it at all (we check later if m_ssa_ranges[x] is non-NULL,
> so we might check it earlier as well) and also in the last loop
> iterates until m_ssa_ranges.length () rather than num_ssa_names, I don't
> see a reason for the inconsistency and in theory some SSA_NAME could be
> added without set_bb_range called for it and the vector could be shorter
> than the ssanames vector.
>
> To actually fix the ICE, either the first hunk or the last 2 hunks
> would be enough, but I think it doesn't hurt to change all the spots.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
> 2023-11-11 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/111967
> * gimple-range-cache.cc (block_range_cache::set_bb_range): Grow
> m_ssa_ranges to num_ssa_names rather than num_ssa_names + 1.
> (block_range_cache::dump): Iterate from 1 rather than 0. Don't use
> ssa_name (x) unless m_ssa_ranges[x] is non-NULL. Iterate to
> m_ssa_ranges.length () rather than num_ssa_names.
>
> * gcc.dg/tree-ssa/pr111967.c: New test.
>
> --- gcc/gimple-range-cache.cc.jj 2023-10-10 11:56:05.819220320 +0200
> +++ gcc/gimple-range-cache.cc 2023-11-10 17:06:52.482867324 +0100
> @@ -390,7 +390,7 @@ block_range_cache::set_bb_range (tree na
> {
> unsigned v = SSA_NAME_VERSION (name);
> if (v >= m_ssa_ranges.length ())
> - m_ssa_ranges.safe_grow_cleared (num_ssa_names + 1);
> + m_ssa_ranges.safe_grow_cleared (num_ssa_names);
>
> if (!m_ssa_ranges[v])
> {
> @@ -465,7 +465,7 @@ void
> block_range_cache::dump (FILE *f)
> {
> unsigned x;
> - for (x = 0; x < m_ssa_ranges.length (); ++x)
> + for (x = 1; x < m_ssa_ranges.length (); ++x)
> {
> if (m_ssa_ranges[x])
> {
> @@ -487,11 +487,14 @@ block_range_cache::dump (FILE *f, basic_
> bool summarize_varying = false;
> for (x = 1; x < m_ssa_ranges.length (); ++x)
> {
> + if (!m_ssa_ranges[x])
> + continue;
> +
> if (!gimple_range_ssa_p (ssa_name (x)))
> continue;
>
> Value_Range r (TREE_TYPE (ssa_name (x)));
> - if (m_ssa_ranges[x] && m_ssa_ranges[x]->get_bb_range (r, bb))
> + if (m_ssa_ranges[x]->get_bb_range (r, bb))
> {
> if (!print_varying && r.varying_p ())
> {
> @@ -508,13 +511,16 @@ block_range_cache::dump (FILE *f, basic_
> if (summarize_varying)
> {
> fprintf (f, "VARYING_P on entry : ");
> - for (x = 1; x < num_ssa_names; ++x)
> + for (x = 1; x < m_ssa_ranges.length (); ++x)
> {
> + if (!m_ssa_ranges[x])
> + continue;
> +
> if (!gimple_range_ssa_p (ssa_name (x)))
> continue;
>
> Value_Range r (TREE_TYPE (ssa_name (x)));
> - if (m_ssa_ranges[x] && m_ssa_ranges[x]->get_bb_range (r, bb))
> + if (m_ssa_ranges[x]->get_bb_range (r, bb))
> {
> if (r.varying_p ())
> {
> --- gcc/testsuite/gcc.dg/tree-ssa/pr111967.c.jj 2023-11-10 16:45:54.006085324
> +0100
> +++ gcc/testsuite/gcc.dg/tree-ssa/pr111967.c 2023-11-10 17:03:17.257844360
> +0100
> @@ -0,0 +1,15 @@
> +/* PR tree-optimization/111967 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fno-tree-forwprop -fdump-tree-evrp-all" } */
> +
> +void bar (char *);
> +int a;
> +char *b;
> +
> +void
> +foo (void)
> +{
> + long c = a & 3;
> + if (c)
> + bar (b + c);
> +}
>
> Jakub
>
