(rsandifo) <[email protected]> requested changes to the code:
How did you test this? When I tried bootstrapping with it locally, I got:
```
In file included from .../gcc/coretypes.h:491,
from .../gcc/emit-rtl.cc:36:
.../gcc/poly-int.h: In instantiation of ‘typename if_nonpoly<Cb, bool>::type
maybe_le(const poly_int<N, C>&, const Cb&) [with unsigned int N = 1; Ca = long
unsigned int; Cb = int; typename if_nonpoly<Cb, bool>::type = bool]’:
.../gcc/poly-int.h:2433:8: required from ‘typename if_nonpoly<Cq, bool>::type
can_div_away_from_zero_p(const poly_int<N, C>&, const poly_int<N, Cb>&, Cq*)
[with unsigned int N = 1; Ca = long unsigned int; Cb = long unsigned int; Cq =
unsigned int; typename if_nonpoly<Cq, bool>::type = bool]’
1351 | #define known_gt(A, B) (!maybe_le (A, B))
| ~~~~~~~~~^~~~~~
.../gcc/emit-rtl.cc:1611:37: required from here
1611 | if (!can_div_away_from_zero_p (msize, regsize, &mregs)
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
.../gcc/poly-int.h:1275:22: error: comparison of integer expressions of
different signedness: ‘const long unsigned int’ and ‘const int’
[-Werror=sign-compare]
1275 | return a.coeffs[0] <= b;
| ~~~~~~~~~~~~^~~~
```
The suggestion below includes a snippet that uses `POLY_INT_TYPE` to avoid
that.
> +++ gcc/poly-int.h
> @@ -2423,0 +2450,4 @@
> + (1) r = 7 + -9 = -2 OR r = -7 + 9 = 2
> + (2) |7| <= |-9| OR |-7| <= |9|
> + (3) |-2| < |-9| OR |2| < |9| */
> + if ((known_lt (a, 0) && known_le (-a, b))
Negating a negative value could lead to signed overflow at the extremes. Also,
the `known_*` conditions aren't trivial, especially for wide-int-based types,
so it would be better not to repeat them if possible.
How about replacing these two blocks with:
```
/* If both a and b have fixed signs, test 0 < |a| <= |b|. (2) is then
satisfied with |Q| == 1. Choose Q as 1 or -1 such that a and b * Q
have the same sign. Together, these two conditions guarantee that
|b * Q - a| = |b * Q| - |a|. Then:
|r| = |a - b * Q|
= |b * Q - a|
= |b * Q| - |a|
= |b| - |a|
< |b| using 0 < |a| above
Only negate values that are known to be positive, to avoid signed
overflow. */
if (known_lt (a, POLY_INT_TYPE (Ca) (0)))
{
if (known_le (b, a))
{
*quotient = 1;
return true;
}
if (known_gt (b, POLY_INT_TYPE (Cb) (0)) && known_le (-b, a))
{
*quotient = -1;
return true;
}
}
else if (known_gt (a, POLY_INT_TYPE (Ca) (0)))
{
if (known_ge (b, a))
{
*quotient = 1;
return true;
}
if (known_lt (b, POLY_INT_TYPE (Cb) (0)) && known_le (b, -a))
{
*quotient = -1;
return true;
}
}
```
I have a feeling there's a more elegant way to write it, but…
> +++ gcc/testsuite/gcc.dg/plugin/poly-int-tests.h
> @@ -2161,6 +2161,14 @@ test_can_div_away_from_zero_p ()
> ph::make (4, 5, 6),
> &const_quot));
> ASSERT_EQ (const_quot, C (0));
> + ASSERT_TRUE (can_div_away_from_zero_p (ph::make (16, 0, 0),
It would be good to test the third coefficient for something other than 0, at
least in some of the tests.
The way the tests work is that the function is instantiated for N==1, N==2, and
N==3. Testing something in the third coefficient would leave the N==2 case
unaffected, i.e. it would still be `[16, 0] / [16, 16]`.
--
https://forge.sourceware.org/gcc/gcc-TEST/pulls/176#issuecomment-6318
