This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>,Marek Polacek <polacek at redhat dot com>
- Cc: Jason Merrill <jason at redhat dot com>,Joseph Myers <joseph at codesourcery dot com>,GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 26 Nov 2014 19:59:41 +0100
- Subject: Re: [C/C++ PATCH] Don't convert RHS of a shift-expression to int (PR c/63862)
- Authentication-results: sourceware.org; auth=none
- References: <20141126173944 dot GW29446 at redhat dot com> <20141126175029 dot GM1669 at tucnak dot redhat dot com> <20141126182004 dot GA15555 at redhat dot com> <20141126184955 dot GO1669 at tucnak dot redhat dot com>
On November 26, 2014 7:49:55 PM CET, Jakub Jelinek <jakub@redhat.com> wrote:
>On Wed, Nov 26, 2014 at 07:20:04PM +0100, Marek Polacek wrote:
>> On Wed, Nov 26, 2014 at 06:50:29PM +0100, Jakub Jelinek wrote:
>> > On Wed, Nov 26, 2014 at 06:39:44PM +0100, Marek Polacek wrote:
>> > > Both C11 and C++14 standards specify that integral promotions are
>> > > performed on both operands of a shift-expression. This we do
>just
>> > > fine. But then we convert the right operand to
>integer_type_node.
>> > > Not only is this unnecessary, it can also be harfmul, because for
>> > > e.g.
>> > > void
>> > > foo (unsigned int x)
>> > > {
>> > > if (-1 >> x != -1)
>
>I'd say fold-const not simplifying this is a bug even if x is signed
>int.
>I think negative shift counts are undefined even in the middle-end,
>Richard?
Well, in the past it has, at least for constant folding, pollibly only in into_const_binop.
>Treating 5 >> -5 as 5 << 5 looks just wrong to me, does any FE rely on
>that?
Not sure.
>I don't think the expander will expand it that way though.
Not sure :)
>Anyway, if the shift count is unnecessary wide type, then the
>middle-end
>won't be able to optimize it as much as it could if it has been a
>narrower
>type.
>
>So, for ubsan purposes (but, shifts are instrumented in the FEs) it is
>of
>course desirable to see the original type, but perhaps it might be a
>good
>idea to cast the shift count to integer_type_node say during
>gimplification
>(perhaps in c_gimplify_expr?).
These are all separate issues but I'd rather get rid of conversions than adding some.
So I think the original patch is OK.
Richard.
> Jakub