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 > > > >