> On 15 Jun 2021, at 00:07, Andrew MacLeod via Gcc-patches
> <[email protected]> wrote:
>
> As mentioned in the Text from the PR:
>
> "When a range is being calculated for an ssa-name, the propagation process
> often goes along back edges. These back edges sometime require other
> ssa-names which have not be processed yet. These are flagged as "poor values"
> and when propagation is done, we visit the list of poor values, calculate
> them, and see if that may result if a better range for the original ssa-name.
>
> The problem is that calculating these poor values may also spawn another set
> of requests since the block at the far end of the back edge has not been
> processed yet... its highly likely that some additional unprocessed ssa-names
> are used in the calculation of that name, but typically they do not affect
> the current range in a significant way.
>
> Thus we mostly we care about the first order effect only. It turns out to be
> very rare that a 2nd order effect on a back edge affects anything that we
> don't catch later.
>
> This patch turns off poor-value tagging when looking up the first order
> values, thus avoiding the 2nd order and beyond cascading effects.
>
> I haven't found a test case we miss yet because of this change, yet it
> probably resolves a number of the outstanding compilation problems in a
> significant way.
>
> I think this will probably apply to gcc 11 in some form as well, so I'll look
> at an equivalent patch for there."
>
>
> This patch simplifies the enable_new_value routines.. replacing the
> enable/disable with an enable with flag routine, which returns the previous
> value.This lets us change the mode and then set it back to what it was
> before. Seems better in general.
>
> Then disables new values for 2nd+ order effects. GCC11 patch forthcoming.
>
> Bootstraps on x86_64-pc-linux-gnu, no regressions. pushed.
>
> Andrew
Hi Andrew,
This causes bootstrap-with-ubsan failure on at least aarch64-linux-gnu, likely,
others:
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 48, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 48, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 32, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 48, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 32, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 48, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 32, which is not a valid value for type 'bool'
# 00:42:32
/home/tcwg-buildslave/workspace/tcwg_gnu_0/abe/snapshots/gcc.git~master/gcc/gimple-range-cache.cc:757:8:
runtime error: load of value 32, which is not a valid value for type 'bool'
>
> @@ -748,21 +748,15 @@ ranger_cache::dump (FILE *f)
> fprintf (f, "\n");
> }
>
> -// Allow the cache to flag and query new values when propagation is forced
> -// to use an unknown value.
> +// Allow or disallow the cache to flag and query new values when propagation
> +// is forced to use an unknown value. The previous state is returned.
>
> -void
> -ranger_cache::enable_new_values ()
> -{
> - m_new_value_p = true;
> -}
> -
> -// Disable new value querying.
> -
> -void
> -ranger_cache::disable_new_values ()
> +bool
> +ranger_cache::enable_new_values (bool state)
> {
> - m_new_value_p = false;
> + bool ret = m_new_value_p;
I think changing this to
bool ret = (bool) m_new_value_p;
might be enough, but you know this code better.
Would you please take a look at this?
> + m_new_value_p = state;
> + return ret;
> }
>
> // Dump the caches for basic block BB to file F.
Thanks,
--
Maxim Kuvyrkov
https://www.linaro.org
