Re: [wide-int] Remove SHIFT_COUNT_TRUNCATED uses at tree/gimple level

[Kenneth Zadeck <zadeck at naturalbridge dot com>]

I very strongly disagree with this.  The standard needs to be high than "does it pass the test suite."

What we are introducing is a case where the program will behave one way with optimization and another way without it.  While, this is always true for timing dependent code, it is pretty rare for math.   We should always tread carefully when doing that, and the excuse that it is "cleaner" does not, in my mind, fit the bill.



On Nov 7, 2013, at 2:08 AM, Richard Sandiford <rdsandiford@googlemail.com> wrote:

> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>> So what is the big plan here?     if you remove it here and then do not 
>> do it in wide int, then it is not going to be truncated.
> 
> Yeah, that is the big plan for trees.  Mainline doesn't truncate at the
> tree level after:
> 
>    http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00285.html
> 
> and this patch brings wide-int back in line.  (The truncations in this patch
> were only ever local to wide-int.)
> 
> If you're wondering why we don't want to truncate at the tree level,
> see the second half of:
> 
>    http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00174.html
> 
> Thanks,
> Richard
> 
>> On 11/06/2013 05:10 PM, Richard Sandiford wrote:
>>> Following the removal of SHIFT_COUNT_TRUNCATED from double-int, this patch
>>> reverts the changed I'd made to mimic the old behaviour on wide-int.
>>> 
>>> Tested on powerpc64-linux-gnu and by assembly comparison on a range of targets.
>>> OK to install?
>>> 
>>> Thanks,
>>> Richard
>>> 
>>> 
>>> Index: gcc/fold-const.c
>>> ===================================================================
>>> --- gcc/fold-const.c    2013-11-05 13:06:56.985255941 +0000
>>> +++ gcc/fold-const.c    2013-11-05 13:12:28.805655903 +0000
>>> @@ -1007,13 +1007,9 @@ int_const_binop_1 (enum tree_code code,
>>>      /* It's unclear from the C standard whether shifts can overflow.
>>>         The following code ignores overflow; perhaps a C standard
>>>         interpretation ruling is needed.  */
>>> -    res = wi::rshift (arg1, arg2, sign,
>>> -              SHIFT_COUNT_TRUNCATED
>>> -              ? GET_MODE_BITSIZE (TYPE_MODE (type)) : 0);
>>> +    res = wi::rshift (arg1, arg2, sign);
>>>        else
>>> -    res = wi::lshift (arg1, arg2,
>>> -              SHIFT_COUNT_TRUNCATED
>>> -              ? GET_MODE_BITSIZE (TYPE_MODE (type)) : 0);
>>> +    res = wi::lshift (arg1, arg2);
>>>        break;
>>> 
>>>      case RROTATE_EXPR:
>>> Index: gcc/tree-ssa-ccp.c
>>> ===================================================================
>>> --- gcc/tree-ssa-ccp.c    2013-11-05 13:07:25.659474362 +0000
>>> +++ gcc/tree-ssa-ccp.c    2013-11-05 13:12:28.806655910 +0000
>>> @@ -1272,20 +1272,15 @@ bit_value_binop_1 (enum tree_code code,
>>>            else
>>>              code = RSHIFT_EXPR;
>>>          }
>>> -          int shift_precision = SHIFT_COUNT_TRUNCATED ? width : 0;
>>>            if (code == RSHIFT_EXPR)
>>>          {
>>> -          *mask = wi::rshift (wi::ext (r1mask, width, sgn),
>>> -                      shift, sgn, shift_precision);
>>> -          *val = wi::rshift (wi::ext (r1val, width, sgn),
>>> -                     shift, sgn, shift_precision);
>>> +          *mask = wi::rshift (wi::ext (r1mask, width, sgn), shift, sgn);
>>> +          *val = wi::rshift (wi::ext (r1val, width, sgn), shift, sgn);
>>>          }
>>>            else
>>>          {
>>> -          *mask = wi::ext (wi::lshift (r1mask, shift, shift_precision),
>>> -                   width, sgn);
>>> -          *val = wi::ext (wi::lshift (r1val, shift, shift_precision),
>>> -                  width, sgn);
>>> +          *mask = wi::ext (wi::lshift (r1mask, shift), width, sgn);
>>> +          *val = wi::ext (wi::lshift (r1val, shift), width, sgn);
>>>          }
>>>          }
>>>      }