On Wed, Mar 9, 2022 at 5:12 PM Xi Ruoyao <xry...@mengyan1223.wang> wrote:
>
> On Wed, 2022-03-09 at 15:55 +0100, Richard Biener wrote:
>
> > isn't it better to make targetm.const_anchor unsigned?
> > The & and ~ are not subject to overflow rules.
>
> It's not enough: if n is the minimum value of HOST_WIDE_INT and
> const_anchor = 0x8000 (the value for MIPS), we'll have a signed 0x7fff
> in *upper_base.  Then the next line, "*upper_offs = n - *upper_base;"
> will be a signed overflow again.
>
> How about the following?

Hmm, so all this seems to be to round CST up and down to a multiple of
CONST_ANCHOR.
It works on CONST_INT only which is sign-extended, so if there is
overflow the resulting
anchor is broken as far as I can see.  So instead of papering over this issue
the function should return false when n is negative since then
n & ~(targetm.const_anchor - 1) is also not n rounded down to a
multiple of const_anchor.

But of course I know nothing about this ..

Richard.

> -- >8 --
>
> With a non-zero const_anchor, the behavior of this function relied on
> signed overflow.
>
> gcc/
>
>         PR rtl-optimization/104843
>         * cse.cc (compute_const_anchors): Use unsigned HOST_WIDE_INT for
>         n to perform overflow arithmetics safely.
> ---
>  gcc/cse.cc | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/cse.cc b/gcc/cse.cc
> index a18b599d324..052fa0c3490 100644
> --- a/gcc/cse.cc
> +++ b/gcc/cse.cc
> @@ -1169,12 +1169,12 @@ compute_const_anchors (rtx cst,
>                        HOST_WIDE_INT *lower_base, HOST_WIDE_INT *lower_offs,
>                        HOST_WIDE_INT *upper_base, HOST_WIDE_INT *upper_offs)
>  {
> -  HOST_WIDE_INT n = INTVAL (cst);
> -
> -  *lower_base = n & ~(targetm.const_anchor - 1);
> -  if (*lower_base == n)
> +  unsigned HOST_WIDE_INT n = UINTVAL (cst);
> +  unsigned HOST_WIDE_INT lb = n & ~(targetm.const_anchor - 1);
> +  if (lb == n)
>      return false;
>
> +  *lower_base = lb;
>    *upper_base =
>      (n + (targetm.const_anchor - 1)) & ~(targetm.const_anchor - 1);
>    *upper_offs = n - *upper_base;
> --
> 2.35.1
>
>
> >

Reply via email to