diff mbox

Put all constants last in tree_swap_operands_p, remove odd -Os check

Message ID alpine.LSU.2.11.1408151100410.20733@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Aug. 15, 2014, 9:07 a.m. UTC
The following makes tree_swap_operands_p put all constants 2nd place,
also looks through sign-changes when considering further canonicalzations
and removes the odd -Os guard for those.  That was put in with
https://gcc.gnu.org/ml/gcc-patches/2003-10/msg01208.html just
motivated by CSiBE numbers - but rather than disabling canonicalization
this should have disabled the actual harmful transforms.

Bootstrap and regtest ongoing on x86_64-unknown-linux-gnu.

Richard.

2014-08-15  Richard Biener  <rguenther@suse.de>

	* fold-const.c (tree_swap_operands_p): Put all constants
	last, also strip sign-changing NOPs when considering further
	canonicalization.  Canonicalize also when optimizing for size.

Comments

Manuel López-Ibáñez Aug. 15, 2014, 9:32 a.m. UTC | #1
On 15 August 2014 11:07, Richard Biener <rguenther@suse.de> wrote:
> -  if (TREE_CODE (arg1) == INTEGER_CST)
> +  if (CONSTANT_CLASS_P (arg1) == INTEGER_CST)

Huh?

/* Nonzero if NODE represents a constant.  */

#define CONSTANT_CLASS_P(NODE)\
    (TREE_CODE_CLASS (TREE_CODE (NODE)) == tcc_constant)

Sadly, we don't have a warning for this, but clang++ has one:

test.c:4:16: warning: comparison of constant 2 with expression of type
'bool' is always false [-Wtautological-constant-out-of-range-compare]
  if ((a == 1) == 2) {
      ~~~~~~~~ ^  ~

I'll open a PR

Cheers,

Manuel.
Richard Biener Aug. 15, 2014, 9:34 a.m. UTC | #2
On Fri, 15 Aug 2014, Manuel López-Ibáñez wrote:

> On 15 August 2014 11:07, Richard Biener <rguenther@suse.de> wrote:
> > -  if (TREE_CODE (arg1) == INTEGER_CST)
> > +  if (CONSTANT_CLASS_P (arg1) == INTEGER_CST)
> 
> Huh?

Eh ;)

> /* Nonzero if NODE represents a constant.  */
> 
> #define CONSTANT_CLASS_P(NODE)\
>     (TREE_CODE_CLASS (TREE_CODE (NODE)) == tcc_constant)
> 
> Sadly, we don't have a warning for this, but clang++ has one:
> 
> test.c:4:16: warning: comparison of constant 2 with expression of type
> 'bool' is always false [-Wtautological-constant-out-of-range-compare]
>   if ((a == 1) == 2) {
>       ~~~~~~~~ ^  ~
> 
> I'll open a PR

Thx.
Richard.
Franz Sirl Aug. 15, 2014, 12:43 p.m. UTC | #3
Am 15.08.2014 um 11:32 schrieb Manuel López-Ibáñez:
> On 15 August 2014 11:07, Richard Biener <rguenther@suse.de> wrote:
>> -  if (TREE_CODE (arg1) == INTEGER_CST)
>> +  if (CONSTANT_CLASS_P (arg1) == INTEGER_CST)
> 
> Huh?
> 
> /* Nonzero if NODE represents a constant.  */
> 
> #define CONSTANT_CLASS_P(NODE)\
>     (TREE_CODE_CLASS (TREE_CODE (NODE)) == tcc_constant)
> 
> Sadly, we don't have a warning for this, but clang++ has one:
> 
> test.c:4:16: warning: comparison of constant 2 with expression of type
> 'bool' is always false [-Wtautological-constant-out-of-range-compare]
>   if ((a == 1) == 2) {
>       ~~~~~~~~ ^  ~
> 
> I'll open a PR

See also PR 44077

Franz
Manuel López-Ibáñez Aug. 15, 2014, 12:52 p.m. UTC | #4
On 15 August 2014 14:43, Franz Sirl <Franz.Sirl-kernel@lauterbach.com> wrote:
> Am 15.08.2014 um 11:32 schrieb Manuel López-Ibáñez:
>> On 15 August 2014 11:07, Richard Biener <rguenther@suse.de> wrote:
>>> -  if (TREE_CODE (arg1) == INTEGER_CST)
>>> +  if (CONSTANT_CLASS_P (arg1) == INTEGER_CST)
>>
>> Huh?
>>
>> /* Nonzero if NODE represents a constant.  */
>>
>> #define CONSTANT_CLASS_P(NODE)\
>>     (TREE_CODE_CLASS (TREE_CODE (NODE)) == tcc_constant)
>>
>> Sadly, we don't have a warning for this, but clang++ has one:
>>
>> test.c:4:16: warning: comparison of constant 2 with expression of type
>> 'bool' is always false [-Wtautological-constant-out-of-range-compare]
>>   if ((a == 1) == 2) {
>>       ~~~~~~~~ ^  ~
>>
>> I'll open a PR
>
> See also PR 44077

Thanks!

I marked it as duplicate since Marek took the other one.

Hopefully this will be fixed for GCC 5
diff mbox

Patch

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 214007)
+++ gcc/fold-const.c	(working copy)
@@ -6642,37 +6650,19 @@  reorder_operands_p (const_tree arg0, con
 bool
 tree_swap_operands_p (const_tree arg0, const_tree arg1, bool reorder)
 {
-  STRIP_SIGN_NOPS (arg0);
-  STRIP_SIGN_NOPS (arg1);
-
-  if (TREE_CODE (arg1) == INTEGER_CST)
+  if (CONSTANT_CLASS_P (arg1) == INTEGER_CST)
     return 0;
-  if (TREE_CODE (arg0) == INTEGER_CST)
+  if (CONSTANT_CLASS_P (arg0) == INTEGER_CST)
     return 1;
 
-  if (TREE_CODE (arg1) == REAL_CST)
-    return 0;
-  if (TREE_CODE (arg0) == REAL_CST)
-    return 1;
-
-  if (TREE_CODE (arg1) == FIXED_CST)
-    return 0;
-  if (TREE_CODE (arg0) == FIXED_CST)
-    return 1;
-
-  if (TREE_CODE (arg1) == COMPLEX_CST)
-    return 0;
-  if (TREE_CODE (arg0) == COMPLEX_CST)
-    return 1;
+  STRIP_NOPS (arg0);
+  STRIP_NOPS (arg1);
 
   if (TREE_CONSTANT (arg1))
     return 0;
   if (TREE_CONSTANT (arg0))
     return 1;
 
-  if (optimize_function_for_size_p (cfun))
-    return 0;
-
   if (reorder && flag_evaluation_order
       && (TREE_SIDE_EFFECTS (arg0) || TREE_SIDE_EFFECTS (arg1)))
     return 0;