This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Cleanup tree-ssa-ter.c exports


On 09/16/2013 10:42 AM, Andrew MacLeod wrote:
On 09/16/2013 04:55 AM, Richard Biener wrote:
On Fri, Sep 13, 2013 at 9:15 PM, Andrew MacLeod <amacleod@redhat.com> wrote:


OK, a slightly different take..
I realized that I should be adding tree-outof-ssa.h to handle the 3 exports from tree-outof-ssa.c that are in ssaexpand.h... In fact, by far the most sensible thing to do is to simply rename tree-outof-ssa.c to ssaexpand.c.
This actually resolves a number of warts... And is_replaceable_p() very
naturally fits in ssaexpand.c now...

what do you think of this option? :-)  and svn rename preserves all the
tree-outof-ssa.c history...
I don't like the new name for tree-outof-ssa.c, it matches less to its contents.

I'd say either keep ssaexpand.h and tree-outof-ssa.c as-is or rename
ssaexpand.h to tree-outof-ssa.h.  I prefer the latter.

I as well.  ssaexpand.h ->tree-outof-ssa.h it is.


The rest of the changes look ok to me, but watch out for odd whitespace
changes:

+static inline bool
+ter_is_replaceable_p (gimple stmt)
+{
+
+  if (ssa_is_replaceable_p (stmt))

spurious vertical space.


bah, where'd that come from :-P.

I'll check this approach in after running it through the gauntlet again.

Andrew
Huh, still in my queue here. For the record, here's the patch I'll was going to apply changes again , (without Makefile.in dependencies), and once I re-verify with a new bootstrap and testsuite runs I'll check it in.

Andrew

	* tree-ssa-live.h (find_replaceable_exprs, dump_replaceable_exprs): Move
	prototypes to...
	* tree-ssa-ter.h: New File.  Move prototypes here.
	* tree-flow.h (stmt_is_replaceable_p): Remove prototype.
	* tree-outof-ssa.h: New. Rename ssaexpand.h, include tree-ssa-ter.h.
	* tree-outof-ssa.c (ssa_is_replaceable_p): New.  Refactor common bits
	from is_replaceable_p. 
	* tree-ssa-ter.c (is_replaceable_p, stmt_is_replaceable_p): Delete.
	(ter_is_replaceable_p): New.  Use new refactored ssa_is_replaceable_p.
	(process_replaceable): Use ter_is_replaceable_p.
	(find_replaceable_in_bb): Use ter_is_replaceable_p.
	* expr.c (stmt_is_replaceable_p): Relocate from tree-ssa-ter.c.  Use
	newly refactored ssa_is_replaceable_p.
	* cfgexpand.c: Include tree-outof-ssa.h.
	* ssaexpand.h: Delete.

Index: tree-ssa-live.h
===================================================================
*** tree-ssa-live.h	(revision 202699)
--- tree-ssa-live.h	(working copy)
*************** make_live_on_entry (tree_live_info_p liv
*** 325,334 ****
  /* From tree-ssa-coalesce.c  */
  extern var_map coalesce_ssa_name (void);
  
- 
- /* From tree-ssa-ter.c  */
- extern bitmap find_replaceable_exprs (var_map);
- extern void dump_replaceable_exprs (FILE *, bitmap);
- 
- 
  #endif /* _TREE_SSA_LIVE_H  */
--- 325,328 ----
Index: tree-ssa-ter.h
===================================================================
*** tree-ssa-ter.h	(revision 0)
--- tree-ssa-ter.h	(revision 0)
***************
*** 0 ****
--- 1,26 ----
+ /* Header file for tree-ssa-ter.c exports.
+    Copyright (C) 2013 Free Software Foundation, Inc.
+ 
+ This file is part of GCC.
+ 
+ GCC is free software; you can redistribute it and/or modify it under
+ the terms of the GNU General Public License as published by the Free
+ Software Foundation; either version 3, or (at your option) any later
+ version.
+ 
+ GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+ WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+  for more details.
+ 
+ You should have received a copy of the GNU General Public License
+ along with GCC; see the file COPYING3.  If not see
+ <http://www.gnu.org/licenses/>.  */
+ 
+ #ifndef GCC_TREE_SSA_TER_H
+ #define GCC_TREE_SSA_TER_H
+ 
+ extern bitmap find_replaceable_exprs (var_map);
+ extern void dump_replaceable_exprs (FILE *, bitmap);
+ 
+ #endif /* GCC_TREE_SSA_TER_H */
Index: tree-flow.h
===================================================================
*** tree-flow.h	(revision 202699)
--- tree-flow.h	(working copy)
*************** bool fixup_noreturn_call (gimple stmt);
*** 683,691 ****
  /* In ipa-pure-const.c  */
  void warn_function_noreturn (tree);
  
- /* In tree-ssa-ter.c  */
- bool stmt_is_replaceable_p (gimple);
- 
  /* In tree-parloops.c  */
  bool parallelized_function_p (tree);
  
--- 683,688 ----
Index: tree-outof-ssa.h
===================================================================
*** tree-outof-ssa.h	(revision 202699)
--- tree-outof-ssa.h	(working copy)
*************** along with GCC; see the file COPYING3.  
*** 22,27 ****
--- 22,28 ----
  #define _SSAEXPAND_H 1
  
  #include "tree-ssa-live.h"
+ #include "tree-ssa-ter.h"
  
  /* This structure (of which only a singleton SA exists) is used to
     pass around information between the outof-SSA functions, cfgexpand
*************** get_gimple_for_ssa_name (tree exp)
*** 71,79 ****
    return NULL;
  }
  
! /* In tree-outof-ssa.c.  */
! void finish_out_of_ssa (struct ssaexpand *sa);
! unsigned int rewrite_out_of_ssa (struct ssaexpand *sa);
! void expand_phi_nodes (struct ssaexpand *sa);
  
  #endif
--- 72,80 ----
    return NULL;
  }
  
! extern bool ssa_is_replaceable_p (gimple stmt);
! extern void finish_out_of_ssa (struct ssaexpand *sa);
! extern unsigned int rewrite_out_of_ssa (struct ssaexpand *sa);
! extern void expand_phi_nodes (struct ssaexpand *sa);
  
  #endif
Index: tree-outof-ssa.c
===================================================================
*** tree-outof-ssa.c	(revision 202699)
--- tree-outof-ssa.c	(working copy)
*************** along with GCC; see the file COPYING3.  
*** 30,41 ****
  #include "tree-ssa.h"
  #include "dumpfile.h"
  #include "diagnostic-core.h"
! #include "ssaexpand.h"
  
  /* FIXME: A lot of code here deals with expanding to RTL.  All that code
     should be in cfgexpand.c.  */
  #include "expr.h"
  
  
  
  /* Used to hold all the components required to do SSA PHI elimination.
--- 30,96 ----
  #include "tree-ssa.h"
  #include "dumpfile.h"
  #include "diagnostic-core.h"
! #include "tree-outof-ssa.h"
  
  /* FIXME: A lot of code here deals with expanding to RTL.  All that code
     should be in cfgexpand.c.  */
  #include "expr.h"
  
+ /* Return TRUE if expression STMT is suitable for replacement.  */
+ 
+ bool
+ ssa_is_replaceable_p (gimple stmt)
+ {
+   use_operand_p use_p;
+   tree def;
+   gimple use_stmt;
+ 
+   /* Only consider modify stmts.  */
+   if (!is_gimple_assign (stmt))
+     return false;
+ 
+   /* If the statement may throw an exception, it cannot be replaced.  */
+   if (stmt_could_throw_p (stmt))
+     return false;
+ 
+   /* Punt if there is more than 1 def.  */
+   def = SINGLE_SSA_TREE_OPERAND (stmt, SSA_OP_DEF);
+   if (!def)
+     return false;
+ 
+   /* Only consider definitions which have a single use.  */
+   if (!single_imm_use (def, &use_p, &use_stmt))
+     return false;
+ 
+   /* Used in this block, but at the TOP of the block, not the end.  */
+   if (gimple_code (use_stmt) == GIMPLE_PHI)
+     return false;
+ 
+   /* There must be no VDEFs.  */
+   if (gimple_vdef (stmt))
+     return false;
+ 
+   /* Float expressions must go through memory if float-store is on.  */
+   if (flag_float_store
+       && FLOAT_TYPE_P (gimple_expr_type (stmt)))
+     return false;
+ 
+   /* An assignment with a register variable on the RHS is not
+      replaceable.  */
+   if (gimple_assign_rhs_code (stmt) == VAR_DECL
+       && DECL_HARD_REGISTER (gimple_assign_rhs1 (stmt)))
+     return false;
+ 
+   /* No function calls can be replaced.  */
+   if (is_gimple_call (stmt))
+     return false;
+ 
+   /* Leave any stmt with volatile operands alone as well.  */
+   if (gimple_has_volatile_ops (stmt))
+     return false;
+ 
+   return true;
+ }
  
  
  /* Used to hold all the components required to do SSA PHI elimination.
Index: tree-ssa-ter.c
===================================================================
*** tree-ssa-ter.c	(revision 202699)
--- tree-ssa-ter.c	(working copy)
*************** along with GCC; see the file COPYING3.  
*** 28,34 ****
  #include "bitmap.h"
  #include "tree-ssa.h"
  #include "dumpfile.h"
! #include "tree-ssa-live.h"
  #include "flags.h"
  
  
--- 28,34 ----
  #include "bitmap.h"
  #include "tree-ssa.h"
  #include "dumpfile.h"
! #include "tree-outof-ssa.h"
  #include "flags.h"
  
  
*************** along with GCC; see the file COPYING3.  
*** 46,52 ****
     information is tracked.
  
     Variables which only have one use, and whose defining stmt is considered
!    a replaceable expression (see is_replaceable_p) are tracked to see whether
     they can be replaced at their use location.
  
     n_12 = C * 10
--- 46,52 ----
     information is tracked.
  
     Variables which only have one use, and whose defining stmt is considered
!    a replaceable expression (see ssa_is_replaceable_p) are tracked to see whether
     they can be replaced at their use location.
  
     n_12 = C * 10
*************** add_dependence (temp_expr_table_p tab, i
*** 359,469 ****
  }
  
  
- /* Return TRUE if expression STMT is suitable for replacement.
-    TER is true if is_replaceable_p is called from within TER, false
-    when used from within stmt_is_replaceable_p, i.e. EXPAND_INITIALIZER
-    expansion.  The differences are that with !TER some tests are skipped
-    to make it more aggressive (doesn't require the same bb, or for -O0
-    same locus and same BLOCK), on the other side never considers memory
-    loads as replaceable, because those don't ever lead into constant
-    expressions.  */
- 
- static inline bool
- is_replaceable_p (gimple stmt, bool ter)
- {
-   use_operand_p use_p;
-   tree def;
-   gimple use_stmt;
-   location_t locus1, locus2;
-   tree block1, block2;
- 
-   /* Only consider modify stmts.  */
-   if (!is_gimple_assign (stmt))
-     return false;
- 
-   /* If the statement may throw an exception, it cannot be replaced.  */
-   if (stmt_could_throw_p (stmt))
-     return false;
- 
-   /* Punt if there is more than 1 def.  */
-   def = SINGLE_SSA_TREE_OPERAND (stmt, SSA_OP_DEF);
-   if (!def)
-     return false;
- 
-   /* Only consider definitions which have a single use.  */
-   if (!single_imm_use (def, &use_p, &use_stmt))
-     return false;
- 
-   /* If the use isn't in this block, it wont be replaced either.  */
-   if (ter && gimple_bb (use_stmt) != gimple_bb (stmt))
-     return false;
- 
-   locus1 = gimple_location (stmt);
-   block1 = LOCATION_BLOCK (locus1);
-   locus1 = LOCATION_LOCUS (locus1);
- 
-   if (gimple_code (use_stmt) == GIMPLE_PHI)
-     locus2 = gimple_phi_arg_location (use_stmt, PHI_ARG_INDEX_FROM_USE (use_p));
-   else
-     locus2 = gimple_location (use_stmt);
-   block2 = LOCATION_BLOCK (locus2);
-   locus2 = LOCATION_LOCUS (locus2);
- 
-   if ((!optimize || optimize_debug)
-       && ter
-       && ((locus1 != UNKNOWN_LOCATION
- 	   && locus1 != locus2)
- 	  || (block1 != NULL_TREE
- 	      && block1 != block2)))
-     return false;
- 
-   /* Used in this block, but at the TOP of the block, not the end.  */
-   if (gimple_code (use_stmt) == GIMPLE_PHI)
-     return false;
- 
-   /* There must be no VDEFs.  */
-   if (gimple_vdef (stmt))
-     return false;
- 
-   /* Without alias info we can't move around loads.  */
-   if ((!optimize || !ter)
-       && gimple_assign_single_p (stmt)
-       && !is_gimple_val (gimple_assign_rhs1 (stmt)))
-     return false;
- 
-   /* Float expressions must go through memory if float-store is on.  */
-   if (flag_float_store
-       && FLOAT_TYPE_P (gimple_expr_type (stmt)))
-     return false;
- 
-   /* An assignment with a register variable on the RHS is not
-      replaceable.  */
-   if (gimple_assign_rhs_code (stmt) == VAR_DECL
-       && DECL_HARD_REGISTER (gimple_assign_rhs1 (stmt)))
-     return false;
- 
-   /* No function calls can be replaced.  */
-   if (is_gimple_call (stmt))
-     return false;
- 
-   /* Leave any stmt with volatile operands alone as well.  */
-   if (gimple_has_volatile_ops (stmt))
-     return false;
- 
-   return true;
- }
- 
- 
- /* Variant of is_replaceable_p test for use in EXPAND_INITIALIZER
-    expansion.  */
- 
- bool
- stmt_is_replaceable_p (gimple stmt)
- {
-   return is_replaceable_p (stmt, false);
- }
- 
- 
  /* This function will remove the expression for VERSION from replacement
     consideration in table TAB.  If FREE_EXPR is true, then remove the
     expression from consideration as well by freeing the decl uid bitmap.  */
--- 359,364 ----
*************** finished_with_expr (temp_expr_table_p ta
*** 487,492 ****
--- 382,443 ----
  }
  
  
+ /* Return TRUE if expression STMT is suitable for replacement.
+    In addition to ssa_is_replaceable_p, require the same bb, and for -O0
+    same locus and same BLOCK), Considers memory loads as replaceable if aliasing
+    is available.  */
+ 
+ static inline bool
+ ter_is_replaceable_p (gimple stmt)
+ {
+ 
+   if (ssa_is_replaceable_p (stmt))
+     {
+       use_operand_p use_p;
+       tree def;
+       gimple use_stmt;
+       location_t locus1, locus2;
+       tree block1, block2;
+ 
+       /* Only consider definitions which have a single use.  ssa_is_replaceable_p
+ 	 already performed this check, but the use stmt pointer is required for
+ 	 further checks.  */
+       def = SINGLE_SSA_TREE_OPERAND (stmt, SSA_OP_DEF);
+       if (!single_imm_use (def, &use_p, &use_stmt))
+ 	  return false;
+ 
+       /* If the use isn't in this block, it wont be replaced either.  */
+       if (gimple_bb (use_stmt) != gimple_bb (stmt))
+         return false;
+ 
+       locus1 = gimple_location (stmt);
+       block1 = LOCATION_BLOCK (locus1);
+       locus1 = LOCATION_LOCUS (locus1);
+ 
+       if (gimple_code (use_stmt) == GIMPLE_PHI)
+ 	locus2 = gimple_phi_arg_location (use_stmt, 
+ 					  PHI_ARG_INDEX_FROM_USE (use_p));
+       else
+ 	locus2 = gimple_location (use_stmt);
+       block2 = LOCATION_BLOCK (locus2);
+       locus2 = LOCATION_LOCUS (locus2);
+ 
+       if ((!optimize || optimize_debug)
+ 	  && ((locus1 != UNKNOWN_LOCATION && locus1 != locus2)
+ 	      || (block1 != NULL_TREE && block1 != block2)))
+ 	return false;
+ 
+       /* Without alias info we can't move around loads.  */
+       if (!optimize && gimple_assign_single_p (stmt)
+ 	  && !is_gimple_val (gimple_assign_rhs1 (stmt)))
+ 	return false;
+ 
+       return true;
+     }
+   return false;
+ }
+ 
+ 
  /* Create an expression entry for a replaceable expression.  */
  
  static void
*************** process_replaceable (temp_expr_table_p t
*** 497,503 ****
    ssa_op_iter iter;
    bitmap def_vars, use_vars;
  
!   gcc_checking_assert (is_replaceable_p (stmt, true));
  
    def = SINGLE_SSA_TREE_OPERAND (stmt, SSA_OP_DEF);
    version = SSA_NAME_VERSION (def);
--- 448,454 ----
    ssa_op_iter iter;
    bitmap def_vars, use_vars;
  
!   gcc_checking_assert (ter_is_replaceable_p (stmt));
  
    def = SINGLE_SSA_TREE_OPERAND (stmt, SSA_OP_DEF);
    version = SSA_NAME_VERSION (def);
*************** find_replaceable_in_bb (temp_expr_table_
*** 612,618 ****
        if (is_gimple_debug (stmt))
  	continue;
  
!       stmt_replaceable = is_replaceable_p (stmt, true);
  
        /* Determine if this stmt finishes an existing expression.  */
        FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE)
--- 563,569 ----
        if (is_gimple_debug (stmt))
  	continue;
  
!       stmt_replaceable = ter_is_replaceable_p (stmt);
  
        /* Determine if this stmt finishes an existing expression.  */
        FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE)
Index: expr.c
===================================================================
*** expr.c	(revision 202699)
--- expr.c	(working copy)
*************** along with GCC; see the file COPYING3.  
*** 49,55 ****
  #include "timevar.h"
  #include "df.h"
  #include "diagnostic.h"
! #include "ssaexpand.h"
  #include "target-globals.h"
  #include "params.h"
  
--- 49,55 ----
  #include "timevar.h"
  #include "df.h"
  #include "diagnostic.h"
! #include "tree-outof-ssa.h"
  #include "target-globals.h"
  #include "params.h"
  
*************** expand_expr_real_2 (sepops ops, rtx targ
*** 9176,9181 ****
--- 9176,9199 ----
  }
  #undef REDUCE_BIT_FIELD
  
+ 
+ /* Return TRUE if expression STMT is suitable for replacement.  
+    Never consider memory loads as replaceable, because those don't ever lead 
+    into constant expressions.  */
+ 
+ static bool
+ stmt_is_replaceable_p (gimple stmt)
+ {
+   if (ssa_is_replaceable_p (stmt))
+     {
+       /* Don't move around loads.  */
+       if (!gimple_assign_single_p (stmt)
+ 	  || is_gimple_val (gimple_assign_rhs1 (stmt)))
+ 	return true;
+     }
+   return false;
+ }
+ 
  rtx
  expand_expr_real_1 (tree exp, rtx target, enum machine_mode tmode,
  		    enum expand_modifier modifier, rtx *alt_rtl)
Index: cfgexpand.c
===================================================================
*** cfgexpand.c	(revision 202699)
--- cfgexpand.c	(working copy)
*************** along with GCC; see the file COPYING3.  
*** 40,46 ****
  #include "tree-inline.h"
  #include "value-prof.h"
  #include "target.h"
! #include "ssaexpand.h"
  #include "bitmap.h"
  #include "sbitmap.h"
  #include "cfgloop.h"
--- 40,46 ----
  #include "tree-inline.h"
  #include "value-prof.h"
  #include "target.h"
! #include "tree-outof-ssa.h"
  #include "bitmap.h"
  #include "sbitmap.h"
  #include "cfgloop.h"
Index: ssaexpand.h
===================================================================
*** ssaexpand.h	(revision 202699)
--- ssaexpand.h	(working copy)
***************
*** 1,79 ****
- /* Routines for expanding from SSA form to RTL.
-    Copyright (C) 2009-2013 Free Software Foundation, Inc.
- 
- This file is part of GCC.
- 
- GCC is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 3, or (at your option)
- any later version.
- 
- GCC is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- GNU General Public License for more details.
- 
- You should have received a copy of the GNU General Public License
- along with GCC; see the file COPYING3.  If not see
- <http://www.gnu.org/licenses/>.  */
- 
- 
- #ifndef _SSAEXPAND_H
- #define _SSAEXPAND_H 1
- 
- #include "tree-ssa-live.h"
- 
- /* This structure (of which only a singleton SA exists) is used to
-    pass around information between the outof-SSA functions, cfgexpand
-    and expand itself.  */
- struct ssaexpand
- {
-   /* The computed partitions of SSA names are stored here.  */
-   var_map map;
- 
-   /* For an SSA name version V bit V is set iff TER decided that
-      its definition should be forwarded.  */
-   bitmap values;
- 
-   /* For a partition number I partition_to_pseudo[I] contains the
-      RTL expression of the allocated space of it (either a MEM or
-      a pseudos REG).  */
-   rtx *partition_to_pseudo;
- 
-   /* If partition I contains an SSA name that has a default def,
-      bit I will be set in this bitmap.  */
-   bitmap partition_has_default_def;
- };
- 
- /* This is the singleton described above.  */
- extern struct ssaexpand SA;
- 
- /* Returns the RTX expression representing the storage of the outof-SSA
-    partition that the SSA name EXP is a member of.  */
- static inline rtx
- get_rtx_for_ssa_name (tree exp)
- {
-   int p = partition_find (SA.map->var_partition, SSA_NAME_VERSION (exp));
-   if (SA.map->partition_to_view)
-     p = SA.map->partition_to_view[p];
-   gcc_assert (p != NO_PARTITION);
-   return SA.partition_to_pseudo[p];
- }
- 
- /* If TER decided to forward the definition of SSA name EXP this function
-    returns the defining statement, otherwise NULL.  */
- static inline gimple
- get_gimple_for_ssa_name (tree exp)
- {
-   int v = SSA_NAME_VERSION (exp);
-   if (SA.values && bitmap_bit_p (SA.values, v))
-     return SSA_NAME_DEF_STMT (exp);
-   return NULL;
- }
- 
- /* In tree-outof-ssa.c.  */
- void finish_out_of_ssa (struct ssaexpand *sa);
- unsigned int rewrite_out_of_ssa (struct ssaexpand *sa);
- void expand_phi_nodes (struct ssaexpand *sa);
- 
- #endif
--- 0 ----

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]