diff mbox

rs6000: Follow up for signed integer overflow fix

Message ID 20141120184143.GK344@x4
State New
Headers show

Commit Message

Markus Trippelsdorf Nov. 20, 2014, 6:41 p.m. UTC 
On 2014.11.20 at 08:59 -0500, David Edelsohn wrote:
> On Thu, Nov 20, 2014 at 8:27 AM, Markus Trippelsdorf
> <markus@trippelsdorf.de> wrote:
> > Running the testsuite after bootstrap-ubsan on gcc112 shows several issues. See
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63426 for the full list.
> >
> > This patch fixes several of them.
> >
> > Tested on powerpc64-unknown-linux-gnu.
> >
> > OK for trunk?
> >
> > Thanks.
> >
> > 2014-11-20  Markus Trippelsdorf  <markus@trippelsdorf.de>
> >
> >         * config/rs6000/constraints.md: Avoid signed integer overflows.
> >         * config/rs6000/predicates.md: Likewise.
> >         * config/rs6000/rs6000.c (num_insns_constant_wide): Likewise.
> >         (includes_rldic_lshift_p): Likewise.
> >         (includes_rldicr_lshift_p): Likewise.
> >         * emit-rtl.c (const_wide_int_htab_hash): Likewise.
> >         * loop-iv.c (determine_max_iter): Likewise.
> >         (iv_number_of_iterations): Likewise.
> >         * tree-ssa-loop-ivopts.c (get_computation_cost_at): Likewise.
> >         * varasm.c (get_section_anchor): Likewise.
> 
> The rs6000 patches are okay.
> 
> Someone like Richi or Jakub needs to approve the changes to the common
> parts of the compiler.

The patch needs a follow up. I have introduced a new compiler warning that I
didn't notice, because I was using --disable-werror during testing
unintentionally.  

Fixed by casting a few 0s to unsigned HOST_WIDE_INT.

Tested with --enable-werror on powerpc64-unknown-linux-gnu.

OK for trunk?

Thanks.

2014-11-20  Markus Trippelsdorf  <markus@trippelsdorf.de>

	* config/rs6000/rs6000.c (includes_rldic_lshift_p): Cast 0 to unsigned.
	(includes_rldicr_lshift_p): Likewise.

Comments

Jakub Jelinek Nov. 20, 2014, 6:44 p.m. UTC | #1 
On Thu, Nov 20, 2014 at 07:41:43PM +0100, Markus Trippelsdorf wrote:
> 2014-11-20  Markus Trippelsdorf  <markus@trippelsdorf.de>
> 
> 	* config/rs6000/rs6000.c (includes_rldic_lshift_p): Cast 0 to unsigned.
> 	(includes_rldicr_lshift_p): Likewise.
> 
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index a9604cf3fa97..d7958b33ba1a 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -16197,10 +16197,10 @@ includes_rldic_lshift_p (rtx shiftop, rtx andop)
>        unsigned HOST_WIDE_INT c, lsb, shift_mask;
>  
>        c = INTVAL (andop);
> -      if (c == 0 || c == ~0)
> +      if (c == 0 || c == ~(unsigned HOST_WIDE_INT) 0)
>  	return 0;
>  
> -      shift_mask = ~0;
> +      shift_mask = ~(unsigned HOST_WIDE_INT) 0;
>        shift_mask <<= INTVAL (shiftop);
>  
>        /* Find the least significant one bit.  */
> @@ -16235,7 +16235,7 @@ includes_rldicr_lshift_p (rtx shiftop, rtx andop)
>      {
>        unsigned HOST_WIDE_INT c, lsb, shift_mask;
>  
> -      shift_mask = ~0;
> +      shift_mask = ~(unsigned HOST_WIDE_INT) 0;
>        shift_mask <<= INTVAL (shiftop);
>        c = INTVAL (andop);

You could use ~HOST_WIDE_INT_UC (0) in all the 3 cases.

	Jakub
Markus Trippelsdorf Nov. 20, 2014, 6:58 p.m. UTC | #2 
On 2014.11.20 at 19:44 +0100, Jakub Jelinek wrote:
> On Thu, Nov 20, 2014 at 07:41:43PM +0100, Markus Trippelsdorf wrote:
> > 2014-11-20  Markus Trippelsdorf  <markus@trippelsdorf.de>
> > 
> > 	* config/rs6000/rs6000.c (includes_rldic_lshift_p): Cast 0 to unsigned.
> > 	(includes_rldicr_lshift_p): Likewise.
> > 
> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> > index a9604cf3fa97..d7958b33ba1a 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -16197,10 +16197,10 @@ includes_rldic_lshift_p (rtx shiftop, rtx andop)
> >        unsigned HOST_WIDE_INT c, lsb, shift_mask;
> >  
> >        c = INTVAL (andop);
> > -      if (c == 0 || c == ~0)
> > +      if (c == 0 || c == ~(unsigned HOST_WIDE_INT) 0)
> >  	return 0;
> >  
> > -      shift_mask = ~0;
> > +      shift_mask = ~(unsigned HOST_WIDE_INT) 0;
> >        shift_mask <<= INTVAL (shiftop);
> >  
> >        /* Find the least significant one bit.  */
> > @@ -16235,7 +16235,7 @@ includes_rldicr_lshift_p (rtx shiftop, rtx andop)
> >      {
> >        unsigned HOST_WIDE_INT c, lsb, shift_mask;
> >  
> > -      shift_mask = ~0;
> > +      shift_mask = ~(unsigned HOST_WIDE_INT) 0;
> >        shift_mask <<= INTVAL (shiftop);
> >        c = INTVAL (andop);
> 
> You could use ~HOST_WIDE_INT_UC (0) in all the 3 cases.

Or better still HOST_WIDE_INT_M1U.
David Edelsohn Nov. 21, 2014, 12:31 a.m. UTC | #3 
On Thu, Nov 20, 2014 at 1:58 PM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> On 2014.11.20 at 19:44 +0100, Jakub Jelinek wrote:
>> On Thu, Nov 20, 2014 at 07:41:43PM +0100, Markus Trippelsdorf wrote:
>> > 2014-11-20  Markus Trippelsdorf  <markus@trippelsdorf.de>
>> >
>> >     * config/rs6000/rs6000.c (includes_rldic_lshift_p): Cast 0 to unsigned.
>> >     (includes_rldicr_lshift_p): Likewise.
>> >
>> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
>> > index a9604cf3fa97..d7958b33ba1a 100644
>> > --- a/gcc/config/rs6000/rs6000.c
>> > +++ b/gcc/config/rs6000/rs6000.c
>> > @@ -16197,10 +16197,10 @@ includes_rldic_lshift_p (rtx shiftop, rtx andop)
>> >        unsigned HOST_WIDE_INT c, lsb, shift_mask;
>> >
>> >        c = INTVAL (andop);
>> > -      if (c == 0 || c == ~0)
>> > +      if (c == 0 || c == ~(unsigned HOST_WIDE_INT) 0)
>> >     return 0;
>> >
>> > -      shift_mask = ~0;
>> > +      shift_mask = ~(unsigned HOST_WIDE_INT) 0;
>> >        shift_mask <<= INTVAL (shiftop);
>> >
>> >        /* Find the least significant one bit.  */
>> > @@ -16235,7 +16235,7 @@ includes_rldicr_lshift_p (rtx shiftop, rtx andop)
>> >      {
>> >        unsigned HOST_WIDE_INT c, lsb, shift_mask;
>> >
>> > -      shift_mask = ~0;
>> > +      shift_mask = ~(unsigned HOST_WIDE_INT) 0;
>> >        shift_mask <<= INTVAL (shiftop);
>> >        c = INTVAL (andop);
>>
>> You could use ~HOST_WIDE_INT_UC (0) in all the 3 cases.
>
> Or better still HOST_WIDE_INT_M1U.

Okay.

Thanks, David
diff mbox

Patch

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index a9604cf3fa97..d7958b33ba1a 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -16197,10 +16197,10 @@  includes_rldic_lshift_p (rtx shiftop, rtx andop)
       unsigned HOST_WIDE_INT c, lsb, shift_mask;
 
       c = INTVAL (andop);
-      if (c == 0 || c == ~0)
+      if (c == 0 || c == ~(unsigned HOST_WIDE_INT) 0)
 	return 0;
 
-      shift_mask = ~0;
+      shift_mask = ~(unsigned HOST_WIDE_INT) 0;
       shift_mask <<= INTVAL (shiftop);
 
       /* Find the least significant one bit.  */
@@ -16235,7 +16235,7 @@  includes_rldicr_lshift_p (rtx shiftop, rtx andop)
     {
       unsigned HOST_WIDE_INT c, lsb, shift_mask;
 
-      shift_mask = ~0;
+      shift_mask = ~(unsigned HOST_WIDE_INT) 0;
       shift_mask <<= INTVAL (shiftop);
       c = INTVAL (andop);