Re: [PATCH] PR tree-optimization/43833

[Jie Zhang <jie at codesourcery dot com>]

Hi Paolo,

Thanks for reviewing.

On 04/21/2010 11:13 PM, Paolo Bonzini wrote:
> On 04/21/2010 04:46 PM, Jie Zhang wrote:
> > + bool vr0_constant_p, vr1_constant_p;
> > +
> > + vr0_constant_p = (vr0.type == VR_RANGE
> > + && vr0.min == vr0.max
>
> operand_equal_p (vr0.min, vr0.max, 0)
I think tree_int_cst_equal is better than operand_equal_p here, right?

> > + && TREE_CODE (vr0.max) == INTEGER_CST
>
> This check then becomes cheaper than operand_equal_p, so do it first.
>
> > + && !TREE_OVERFLOW (vr0.max));
> > + vr1_constant_p = (vr1.type == VR_RANGE
> > + && vr1.min == vr1.max
> > + && TREE_CODE (vr1.max) == INTEGER_CST
> > + && !TREE_OVERFLOW (vr1.max));
> > +
> > + if (vr0_constant_p&& vr1_constant_p)
>
> I also looked at op_with_constant_singleton_value_range while reviewing
> this patch, and noticed it lacks the !TREE_OVERFLOW check. Maybe you can
> introduce range_single_constant_value, and use it also in
> op_with_constant_singleton_value_range?
I don't want to touch op_with_constant_singleton_value_range in this 
patch. I want to port this patch to stable branches so I'd better avoid 
adding unnecessary changes for this PR.

Anyway I tried to write range_single_constant_value but just found 
op_with_constant_singleton_value_range wanted not just INTEGER_CST while 
I only have to handle INTEGER_CST here for BIT_AND_EXPR. I cannot write 
a pretty function to service both purposes.

But I also found that BIT_IOR_EXPR case was lack of TREE_OVERFLOW check. 
And both BIT_AND_EXPR and BIT_IOR_EXPR care about INTEGER_CST. So I 
think it's a good idea to make them share some checks, i.e. 
range_int_cst_p and range_int_cst_singleton_p in this new patch.

This new patch also uses int_const_binop for BIT_AND_EXPR folding 
instead of doing it itself.

Bootstrapped and regtested on x86_64-pc-linux-gnu. Is it OK?


--
Jie Zhang
CodeSourcery
(650) 331-3385 x735

	PR 43562
	* reload.h (caller_save_initialized_p): Declare.
	* toplev.c (backend_init_target): Don't call
	init_caller_save but set caller_save_initialized_p
	to false.
	* caller-save.c (caller_save_initialized_p): Define.
	(init_caller_save): Check caller_save_initialized_p.
	* ira.c (ira): Call init_caller_save if flag_caller_saves.

	testsuite/
	PR 43562
	* gcc.dg/pr43562.c: New test.

Index: reload.h
===================================================================
--- reload.h	(revision 157794)
+++ reload.h	(working copy)
@@ -349,6 +349,9 @@ extern bool elimination_target_reg_p (rt
 /* Deallocate the reload register used by reload number R.  */
 extern void deallocate_reload_reg (int r);
 
+/* True if caller-save has been reinitialized.  */
+extern bool caller_save_initialized_p;
+
 /* Functions in caller-save.c:  */
 
 /* Initialize for caller-save.  */
Index: toplev.c
===================================================================
--- toplev.c	(revision 157795)
+++ toplev.c	(working copy)
@@ -2191,8 +2191,8 @@ backend_init_target (void)
 
   /* We may need to recompute regno_save_code[] and regno_restore_code[]
      after a mode change as well.  */
-  if (flag_caller_saves)
-    init_caller_save ();
+  caller_save_initialized_p = false;
+
   expand_dummy_function_end ();
 }
 
Index: caller-save.c
===================================================================
--- caller-save.c	(revision 157794)
+++ caller-save.c	(working copy)
@@ -40,6 +40,9 @@ along with GCC; see the file COPYING3.
 #include "df.h"
 #include "ggc.h"
 
+/* True if caller-save has been initialized.  */
+bool caller_save_initialized_p;
+
 /* Call used hard registers which can not be saved because there is no
    insn for this.  */
 HARD_REG_SET no_caller_save_reg_set;
@@ -208,6 +211,11 @@ init_caller_save (void)
   rtx address;
   int i, j;
 
+  if (caller_save_initialized_p)
+    return;
+
+  caller_save_initialized_p = true;
+
   CLEAR_HARD_REG_SET (no_caller_save_reg_set);
   /* First find all the registers that we need to deal with and all
      the modes that they can have.  If we can't find a mode to use,
Index: ira.c
===================================================================
--- ira.c	(revision 157794)
+++ ira.c	(working copy)
@@ -3127,6 +3127,9 @@ ira (FILE *f)
 
   timevar_push (TV_IRA);
 
+  if (flag_caller_saves)
+    init_caller_save ();
+
   if (flag_ira_verbose < 10)
     {
       internal_flag_ira_verbose = flag_ira_verbose;
Index: testsuite/gcc.dg/pr43562.c
===================================================================
--- testsuite/gcc.dg/pr43562.c	(revision 0)
+++ testsuite/gcc.dg/pr43562.c	(revision 0)
@@ -0,0 +1,20 @@
+/* { dg-options "-O0" } */
+/* { dg-do compile } */
+
+extern unsigned foo (void);
+extern void bar (void);
+
+__attribute__ ((optimize ("O2")))
+void bak ()
+{
+  unsigned a;
+  while (1)
+    {
+      a = foo ();
+      while (a)
+	{
+	  a &= 1;
+	  bar ();
+	}
+    }
+}