[PATCH][RFC] Minuscule patch to remove apparently accidental break from switch

[PATCH][RFC] Minuscule patch to remove apparently accidental break from switch

[Simon Baldwin]

An apparently misplaced break statement within simplify_binary_operation_1()
causes calls to simplify_associative_operation() for xor operations to
potentially be missed.

This patch removes the break, enabling simplify_associative_operation().

Regression tested on i386 against the full g++ testsuite.


gcc/ChangeLog
2007-06-05  Simon Baldwin  <simonb@google.com>

	* simplify-rtx.c (simplify_binary_operation_1): Removed erroneous
	break that was preventing simplify_associative_operation() for xor.


diff -Nrcp3 orig/gcc/gcc/simplify-rtx.c gcc/gcc/simplify-rtx.c
*** orig/gcc/gcc/simplify-rtx.c	Mon Jun  4 09:49:07 2007
--- gcc/gcc/simplify-rtx.c	Tue Jun  5 10:52:40 2007
*************** simplify_binary_operation_1 (enum rtx_co
*** 2292,2299 ****
  	  && (reversed = reversed_comparison (op0, mode)))
  	return reversed;
  
-       break;
-       
        tem = simplify_associative_operation (code, mode, op0, op1);
        if (tem)
  	return tem;

Re: [PATCH][RFC] Minuscule patch to remove apparently accidental break from switch

[Andrew Pinski]

On 6/5/07, Simon Baldwin <simonb@google.com> wrote:
> An apparently misplaced break statement within simplify_binary_operation_1()
> causes calls to simplify_associative_operation() for xor operations to
> potentially be missed.
>
> This patch removes the break, enabling simplify_associative_operation().
>
> Regression tested on i386 against the full g++ testsuite.

Can you run against the whole testsuite instead of just the g++?  This
patch really needs a full bootstrap with all languages enabled as it
changes the middle-end.

Thanks,
Andrew Pinski

Re: [PATCH][RFC] Minuscule patch to remove apparently accidental break from switch

[Simon Baldwin]

Andrew Pinski wrote:

> On 6/5/07, Simon Baldwin <simonb@google.com> wrote:
>
>> An apparently misplaced break statement within 
>> simplify_binary_operation_1()
>> causes calls to simplify_associative_operation() for xor operations to
>> potentially be missed.
>>
>> This patch removes the break, enabling simplify_associative_operation().
>>
>> Regression tested on i386 against the full g++ testsuite.
>
>
> Can you run against the whole testsuite instead of just the g++?  This
> patch really needs a full bootstrap with all languages enabled as it
> changes the middle-end.
>

Thanks for the note.  That's actually my typo in the patch description; 
I've actually run both C and C++ testsuites.  That is, all tests enabled 
by "configure --enable-languages=c,c++".

Will those cover it, or should I plough through Fortran, ADA, and so on 
as well?

Thanks,

--S

Re: [PATCH][RFC] Minuscule patch to remove apparently accidental break from switch

[David Daney]

Simon Baldwin wrote:
> Andrew Pinski wrote:
> 
>> On 6/5/07, Simon Baldwin <simonb@google.com> wrote:
>>
>>> An apparently misplaced break statement within 
>>> simplify_binary_operation_1()
>>> causes calls to simplify_associative_operation() for xor operations to
>>> potentially be missed.
>>>
>>> This patch removes the break, enabling simplify_associative_operation().
>>>
>>> Regression tested on i386 against the full g++ testsuite.
>>
>>
>> Can you run against the whole testsuite instead of just the g++?  This
>> patch really needs a full bootstrap with all languages enabled as it
>> changes the middle-end.
>>
> 
> Thanks for the note.  That's actually my typo in the patch description; 
> I've actually run both C and C++ testsuites.  That is, all tests enabled 
> by "configure --enable-languages=c,c++".
> 
> Will those cover it, or should I plough through Fortran, ADA, and so on 
> as well?

Just remove the --enable-languages and test all default languages.

David Daney

Re: [PATCH][RFC] Minuscule patch to remove apparently accidental break from switch

[Arnaud Charlet]

> >Will those cover it, or should I plough through Fortran, ADA, and so on 
> >as well?
> 
> Just remove the --enable-languages and test all default languages.

To answer the question about Ada, to enable all languages, including Ada,
you can do: --enable-languages=all,ada

You do not have to, but you are encouraged to, so thanks in advance if
you do.

Arno

Re: [PATCH][RFC] Minuscule patch to remove apparently accidental break from switch

[Simon Baldwin]

Arnaud Charlet wrote:

>>>Will those cover it, or should I plough through Fortran, ADA, and so on 
>>>as well?
>>>      
>>>
>>Just remove the --enable-languages and test all default languages.
>>    
>>
>
>To answer the question about Ada, to enable all languages, including Ada,
>you can do: --enable-languages=all,ada
>
>You do not have to, but you are encouraged to, so thanks in advance if
>you do.
>  
>

I've finally got around to rebuilding and retesting for all languages, 
including Ada, objective C, and Fortran, with this patch.  There is no 
difference in test results between the two.

Okay for trunk, then?
 


BTW, as a refresher, since it's been a while, here's the totality of the 
patch:

$ svn diff simplify-rtx.c
Index: simplify-rtx.c
===================================================================
--- simplify-rtx.c      (revision 127106)
+++ simplify-rtx.c      (working copy)
@@ -2293,8 +2293,6 @@
          && (reversed = reversed_comparison (op0, mode)))
        return reversed;

-      break;
-
       tem = simplify_associative_operation (code, mode, op0, op1);
       if (tem)
        return tem;

Re: [PATCH][RFC] Minuscule patch to remove apparently accidental break from switch

[Diego Novillo]

On 8/1/07 7:40 PM, Simon Baldwin wrote:

> Okay for trunk, then?

OK with ChangeLog.