This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Thu, 7 Nov 2013, Kenneth Zadeck wrote:

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

It happens always for code with undefined behavior.  See the gazillions
of INVALID bugreports that complain that -O0 produces different results
than -On.

Patch is ok.

Thanks,
Richard. 

> 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);
> >>>          }
> >>>          }
> >>>      }
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]