Hi,

This patch fixes the bug PR43491, which exists at least on
arm-none-eabi/mips-elf
targets.

The cause is ssa-pre eliminates global register variable when it is the RHS
of
single assign statment, while following passes do not handle the
const/register
attributes of the variable.

This patch skips the elimination of global register variable when it is the
RHS
of a single assignment in SSA-PRE pass.

By doing this,

0) gcc won't generate the redundant move instruction as reported in the bug.
1) normal redundancy elimination on global registers will not be hurt, 
   since sccvn and pre has already detected the true elimination chances 
   and they will be eliminated afterward in function eliminate.
2) the inserted statements(including PHIs) for global register variables
   will not be marked as NECESSARY in function eliminate and will be
   deleted in remove_dead_inserted_code. So no redundant insertions will be
   generated in PRE pass.

Some discussion can be found at:
  http://gcc.gnu.org/ml/gcc/2011-12/msg00000.html

The patch is tested on x86 and arm-none-eabi, no failure introduced.

Is it OK?

Thanks

gcc/ChangeLog:
2011-12-21  Bin Cheng  <bin.ch...@arm.com>
            Richard Guenther  <rguent...@suse.de>

        PR tree-optimization/43491
        * tree-ssa-pre.c (eliminate): Don't replace global register variable
when 
        it is the RHS of a single assign.

gcc/testsuite/ChangeLog:
2011-12-21  Bin Cheng  <bin.ch...@arm.com>

        PR tree-optimization/43491
        * gcc.dg/tree-ssa/pr43491.c: New test.
Index: gcc/testsuite/gcc.dg/tree-ssa/pr43491.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/pr43491.c     (revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr43491.c     (revision 0)
@@ -0,0 +1,42 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-pre-stats" } */
+
+#define REGISTER register
+
+#if defined __arm__
+# define REG1 asm("r4")
+#elif defined __i386__
+# define REG1 asm("ebx")
+#elif defined __mips__
+# define REG1 asm("s0")
+#elif defined __x86_64__
+# define REG1 asm("rbp")
+#else
+# undef REGISTER
+# define REGISTER
+# define REG1
+#endif
+
+REGISTER long data_0 REG1;
+long data_3; 
+
+long foo(long data, long v)
+{
+       long i;
+       long t, u;
+
+       if (data)
+               i = data_0 + data_3;
+       else {
+               v = 2;
+               i = 5;
+       }
+       t = data_0 + data_3;
+       u = i;
+       return v * t * u;
+}
+/* We should not eliminate global register variable when it is the RHS of
+   a single assignment.  */
+/* { dg-final { scan-tree-dump-times "Eliminated: 2" 1 "pre" { target { 
arm-*-* i?86-*-* mips*-*-* x86_64-*-* } } } } */
+/* { dg-final { scan-tree-dump-times "Eliminated: 3" 1 "pre" { target { ! { 
arm-*-* i?86-*-* mips*-*-* x86_64-*-* } } } } } */
+/* { dg-final { cleanup-tree-dump "pre" } } */
Index: gcc/tree-ssa-pre.c
===================================================================
--- gcc/tree-ssa-pre.c  (revision 182075)
+++ gcc/tree-ssa-pre.c  (working copy)
@@ -4161,28 +4161,40 @@
     {
       for (gsi = gsi_start_bb (b); !gsi_end_p (gsi); gsi_next (&gsi))
        {
+         tree lhs = NULL_TREE;
+         tree rhs = NULL_TREE;
+
          stmt = gsi_stmt (gsi);
 
+         if (gimple_has_lhs (stmt))
+           lhs = gimple_get_lhs (stmt);
+
+         if (gimple_assign_single_p (stmt))
+           rhs = gimple_assign_rhs1 (stmt);
+
          /* Lookup the RHS of the expression, see if we have an
             available computation for it.  If so, replace the RHS with
-            the available computation.  */
+            the available computation.
+
+            See PR43491.
+            We don't replace global register variable when it is a the RHS of
+            a single assign. We do replace local register variable since gcc
+            does not guarantee local variable will be allocated in register.  
*/
          if (gimple_has_lhs (stmt)
-             && TREE_CODE (gimple_get_lhs (stmt)) == SSA_NAME
+             && TREE_CODE (lhs) == SSA_NAME
              && !gimple_assign_ssa_name_copy_p (stmt)
              && (!gimple_assign_single_p (stmt)
-                 || !is_gimple_min_invariant (gimple_assign_rhs1 (stmt)))
+                 || (!is_gimple_min_invariant (rhs)
+                      && (gimple_assign_rhs_code (stmt) != VAR_DECL
+                          || !is_global_var (rhs)
+                          || !DECL_HARD_REGISTER (rhs))))
              && !gimple_has_volatile_ops  (stmt)
-             && !has_zero_uses (gimple_get_lhs (stmt)))
+             && !has_zero_uses (lhs))
            {
-             tree lhs = gimple_get_lhs (stmt);
-             tree rhs = NULL_TREE;
              tree sprime = NULL;
              pre_expr lhsexpr = get_or_alloc_expr_for_name (lhs);
              pre_expr sprimeexpr;
 
-             if (gimple_assign_single_p (stmt))
-               rhs = gimple_assign_rhs1 (stmt);
-
              sprimeexpr = bitmap_find_leader (AVAIL_OUT (b),
                                               get_expr_value_id (lhsexpr),
                                               NULL);
@@ -4298,10 +4310,9 @@
             dead.  */
          else if (gimple_assign_single_p (stmt)
                   && !is_gimple_reg (gimple_assign_lhs (stmt))
-                  && (TREE_CODE (gimple_assign_rhs1 (stmt)) == SSA_NAME
-                      || is_gimple_min_invariant (gimple_assign_rhs1 (stmt))))
+                  && (TREE_CODE (rhs) == SSA_NAME
+                      || is_gimple_min_invariant (rhs)))
            {
-             tree rhs = gimple_assign_rhs1 (stmt);
              tree val;
              val = vn_reference_lookup (gimple_assign_lhs (stmt),
                                         gimple_vuse (stmt), VN_WALK, NULL);

Reply via email to