On Wed, Mar 9, 2022 at 5:12 PM Xi Ruoyao <[email protected]> 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
>
>
> >
