mpz_sqrt_if_perfect_square

[Niels Möller]

Seth Troisi <braintwo at gmail.com> writes:

> I changed the name.
> Would mpn_probab_perfect_square_p (no ly) better match mpz_probab_prime?

It would. Any second opinions on naming?

>> >  @deftypefun void mpn_and_n (mp_limb_t *@var{rp}, const mp_limb_t
>> *@var{s1p}, const mp_limb_t *@var{s2p}, mp_size_t @var{n})
>> > @@ -9250,9 +9263,10 @@
>> >  A significant fraction of non-squares can be quickly identified by
>> checking
>> >  whether the input is a quadratic residue modulo small integers.
>> ...
>> I think these docs should be attached to mpn_perfect_square_trivial_p.
>>
>
> I'd prefer not to be the person making the decision on where these notes
> (currently in algorithm) go.
> Would it be reasonable for any changes to be made after this patch is
> merged?

It can be adjusted later (and I missed a bit of the structure when
reading the patch, I didn't realize the text is currently in the
algorithms section).

> I updated the comment to
>       /* It might be better to always copy so the value in rop doesn't
>          depend on if the arguments are aliased or not. */
>
> I could always allocate the temporary but I think that's less efficient

If docs say that value is undefined, then I don't think it's worth an
allocation to make it have some particular value. 

If I understand your code correctly, then in the non-square case, rop
will either be unchanged or get floor (sqrt(op)), depending on result of
the initial check and on whether op == rop.

>> > +  if (res)
>> > +    {
>> > +      SIZ(rop) = rop_size;
>> > +    }
>> > +  return res;
>
> I changed this to always set SIZ (rop) = rop_size so rop is the return of
> sqrt(op).

Looks good.

> As an interesting thought If the contract was inverted (return zero if
> perfect square) This could return
> 0: perfect square, rop set to sqrt
> 1: not square, rop set to sqrt
> 2: not square, rop not set

Seems a bit questionable to me; if one has some need for the sqrt both
in square and non-square cases, why not just call mpz_sqrtrem?

> I guess if someone needs this functionality right now they can set rop to
> any negative sentinel.

Or use the new probab_square_p function.

Overall, I think this looks really good. Thanks!

Regards,
/Niels

-- 
Niels Möller. PGP key CB4962D070D77D7FCB8BA36271D8F1FF368C6677.
Internet email is subject to wholesale government surveillance.