Fix up wi::lrshift (PR c++/69399)

On Tue, Jan 26, 2016 at 11:00:52AM -0800, Mike Stump wrote:
> On Jan 26, 2016, at 10:21 AM, Jakub Jelinek <jakub@redhat.com> wrote
> > The question is, shall we do what wi::lshift does and have the fast path
> > only for the constant shifts, as the untested patch below does, or shall we
> > check shift dynamically (thus use
> > shift < HOST_BITS_PER_WIDE_INT
> > instead of
> > STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT)
> > in the patch),
> 
> Hum…  I think I prefer the dynamic check.  The reasoning is that when we
> fast path, we can tolerate the conditional branch to retain the fast path,
> as most of the time, that condition will usually be true.  If the compiler
> had troubles with knowing the usual truth value of the expression, seems
> like we can hint that it will be true and influence the static prediction
> of the branch.  This permits us to fast path almost all the time in the
> non-constant, but small enough case.  For known shifts, there is no code
> gen difference, so it doesn’t matter.

Ok, I've cancelled my pending bootstrap/regtest and am testing this instead:

2016-01-26  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/69399
	* wide-int.h (wi::lrshift): For larger precisions, only
	use fast path if shift is known to be < HOST_BITS_PER_WIDE_INT.



	Jakub

On Tue, Jan 26, 2016 at 11:17 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Tue, Jan 26, 2016 at 11:00:52AM -0800, Mike Stump wrote:
>> On Jan 26, 2016, at 10:21 AM, Jakub Jelinek <jakub@redhat.com> wrote
>> > The question is, shall we do what wi::lshift does and have the fast path
>> > only for the constant shifts, as the untested patch below does, or shall we
>> > check shift dynamically (thus use
>> > shift < HOST_BITS_PER_WIDE_INT
>> > instead of
>> > STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT)
>> > in the patch),
>>
>> Hum…  I think I prefer the dynamic check.  The reasoning is that when we
>> fast path, we can tolerate the conditional branch to retain the fast path,
>> as most of the time, that condition will usually be true.  If the compiler
>> had troubles with knowing the usual truth value of the expression, seems
>> like we can hint that it will be true and influence the static prediction
>> of the branch.  This permits us to fast path almost all the time in the
>> non-constant, but small enough case.  For known shifts, there is no code
>> gen difference, so it doesn’t matter.
>
> Ok, I've cancelled my pending bootstrap/regtest and am testing this instead:
>
> 2016-01-26  Jakub Jelinek  <jakub@redhat.com>
>
>         PR tree-optimization/69399
>         * wide-int.h (wi::lrshift): For larger precisions, only
>         use fast path if shift is known to be < HOST_BITS_PER_WIDE_INT.
>
> --- gcc/wide-int.h.jj   2016-01-04 18:50:34.656471663 +0100
> +++ gcc/wide-int.h      2016-01-26 20:07:03.147054988 +0100
> @@ -2909,7 +2909,9 @@ wi::lrshift (const T1 &x, const T2 &y)
>          For variable-precision integers like wide_int, handle HWI
>          and sub-HWI integers inline.  */
>        if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT)
> -         ? xi.len == 1 && xi.val[0] >= 0
> +         ? (shift < HOST_BITS_PER_WIDE_INT
> +            && xi.len == 1
> +            && xi.val[0] >= 0)
>           : xi.precision <= HOST_BITS_PER_WIDE_INT)
>         {
>           val[0] = xi.to_uhwi () >> shift;
>
>
>         Jakub

Can you add the testcase in PR 69399 to gcc.dg/torture?

Thanks.

Jakub Jelinek <jakub@redhat.com> writes:
> On Tue, Jan 26, 2016 at 11:00:52AM -0800, Mike Stump wrote:
>> On Jan 26, 2016, at 10:21 AM, Jakub Jelinek <jakub@redhat.com> wrote
>> > The question is, shall we do what wi::lshift does and have the fast path
>> > only for the constant shifts, as the untested patch below does, or shall we
>> > check shift dynamically (thus use
>> > shift < HOST_BITS_PER_WIDE_INT
>> > instead of
>> > STATIC_CONSTANT_P (shift < HOST_BITS_PER_WIDE_INT)
>> > in the patch),
>> 
>> Hum…  I think I prefer the dynamic check.  The reasoning is that when we
>> fast path, we can tolerate the conditional branch to retain the fast path,
>> as most of the time, that condition will usually be true.  If the compiler
>> had troubles with knowing the usual truth value of the expression, seems
>> like we can hint that it will be true and influence the static prediction
>> of the branch.  This permits us to fast path almost all the time in the
>> non-constant, but small enough case.  For known shifts, there is no code
>> gen difference, so it doesn’t matter.
>
> Ok, I've cancelled my pending bootstrap/regtest and am testing this instead:
>
> 2016-01-26  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR tree-optimization/69399
> 	* wide-int.h (wi::lrshift): For larger precisions, only
> 	use fast path if shift is known to be < HOST_BITS_PER_WIDE_INT.
>
> --- gcc/wide-int.h.jj	2016-01-04 18:50:34.656471663 +0100
> +++ gcc/wide-int.h	2016-01-26 20:07:03.147054988 +0100
> @@ -2909,7 +2909,9 @@ wi::lrshift (const T1 &x, const T2 &y)
>  	 For variable-precision integers like wide_int, handle HWI
>  	 and sub-HWI integers inline.  */
>        if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT)
> -	  ? xi.len == 1 && xi.val[0] >= 0
> +	  ? (shift < HOST_BITS_PER_WIDE_INT
> +	     && xi.len == 1
> +	     && xi.val[0] >= 0)
>  	  : xi.precision <= HOST_BITS_PER_WIDE_INT)
>  	{
>  	  val[0] = xi.to_uhwi () >> shift;

LGTM, thanks.

Richard