diff mbox series

Fix PR middle-end/82569

Message ID 6561954.cUVdaKEquF@polaris
State New
Headers show
Series Fix PR middle-end/82569 | expand

Commit Message

Eric Botcazou Oct. 23, 2017, 10:57 a.m. UTC
Hi,

this is the regression present on the mainline for Power6 and introduced by my 
patch fiddling with SUBREG_PROMOTED_VAR_P in expand_expr_real_1.  It turns out 
that the ouf-of-ssa pass implicitly assumes that promoted RTXes for partitions 
are fully initialized (because it can generate direct moves in promoted mode) 
and clearing SUBREG_PROMOTED_VAR_P for some of them goes against this.

Therefore the attached patch goes in the opposite direction and initializes 
the RTXes for problematic partitions on function entry.  Surprisingly enough, 
this generates smaller code on average at -O2 for gcc.c-torture/compile:

 49 files changed, 1243 insertions(+), 1694 deletions(-)

probably because the compiler can now infer values on paths from entry where 
variables are uninitialized.  Tested on PowerPC64/Linux, OK for the mainline?


2017-10-23  Eric Botcazou  <ebotcazou@adacore.com>

	PR middle-end/82569
	* tree-outof-ssa.h (always_initialized_rtx_for_ssa_name_p): Delete.
	* expr.c (expand_expr_real_1) <expand_decl_rtl>: Revert latest change.
	* loop-iv.c (iv_get_reaching_def): Likewise.
	* cfgexpand.c (expand_one_ssa_partition): Initialize the RTX if the 
	variable is promoted and the partition contains undefined values.

Comments

Richard Biener Oct. 23, 2017, 11:07 a.m. UTC | #1
On Mon, Oct 23, 2017 at 12:57 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this is the regression present on the mainline for Power6 and introduced by my
> patch fiddling with SUBREG_PROMOTED_VAR_P in expand_expr_real_1.  It turns out
> that the ouf-of-ssa pass implicitly assumes that promoted RTXes for partitions
> are fully initialized (because it can generate direct moves in promoted mode)
> and clearing SUBREG_PROMOTED_VAR_P for some of them goes against this.
>
> Therefore the attached patch goes in the opposite direction and initializes
> the RTXes for problematic partitions on function entry.  Surprisingly enough,
> this generates smaller code on average at -O2 for gcc.c-torture/compile:
>
>  49 files changed, 1243 insertions(+), 1694 deletions(-)
>
> probably because the compiler can now infer values on paths from entry where
> variables are uninitialized.  Tested on PowerPC64/Linux, OK for the mainline?

Ok.

Thanks,
Richard.


>
> 2017-10-23  Eric Botcazou  <ebotcazou@adacore.com>
>
>         PR middle-end/82569
>         * tree-outof-ssa.h (always_initialized_rtx_for_ssa_name_p): Delete.
>         * expr.c (expand_expr_real_1) <expand_decl_rtl>: Revert latest change.
>         * loop-iv.c (iv_get_reaching_def): Likewise.
>         * cfgexpand.c (expand_one_ssa_partition): Initialize the RTX if the
>         variable is promoted and the partition contains undefined values.
>
> --
> Eric Botcazou
diff mbox series

Patch

Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 253968)
+++ cfgexpand.c	(working copy)
@@ -1391,10 +1391,18 @@  expand_one_ssa_partition (tree var)
     }
 
   machine_mode reg_mode = promote_ssa_mode (var, NULL);
-
   rtx x = gen_reg_rtx (reg_mode);
 
   set_rtl (var, x);
+
+  /* For a promoted variable, X will not be used directly but wrapped in a
+     SUBREG with SUBREG_PROMOTED_VAR_P set, which means that the RTL land
+     will assume that its upper bits can be inferred from its lower bits.
+     Therefore, if X isn't initialized on every path from the entry, then
+     we must do it manually in order to fulfill the above assumption.  */
+  if (reg_mode != TYPE_MODE (TREE_TYPE (var))
+      && bitmap_bit_p (SA.partitions_for_undefined_values, part))
+    emit_move_insn (x, CONST0_RTX (reg_mode));
 }
 
 /* Record the association between the RTL generated for partition PART
Index: expr.c
===================================================================
--- expr.c	(revision 253968)
+++ expr.c	(working copy)
@@ -9912,43 +9912,24 @@  expand_expr_real_1 (tree exp, rtx target
 	  && GET_MODE (decl_rtl) != dmode)
 	{
 	  machine_mode pmode;
-	  bool always_initialized_rtx;
 
 	  /* Get the signedness to be used for this variable.  Ensure we get
 	     the same mode we got when the variable was declared.  */
 	  if (code != SSA_NAME)
-	    {
-	      pmode = promote_decl_mode (exp, &unsignedp);
-	      always_initialized_rtx = true;
-	    }
+	    pmode = promote_decl_mode (exp, &unsignedp);
 	  else if ((g = SSA_NAME_DEF_STMT (ssa_name))
 		   && gimple_code (g) == GIMPLE_CALL
 		   && !gimple_call_internal_p (g))
-	    {
-	      pmode = promote_function_mode (type, mode, &unsignedp,
-					    gimple_call_fntype (g), 2);
-	      always_initialized_rtx
-		= always_initialized_rtx_for_ssa_name_p (ssa_name);
-	    }
+	    pmode = promote_function_mode (type, mode, &unsignedp,
+					   gimple_call_fntype (g),
+					   2);
 	  else
-	    {
-	      pmode = promote_ssa_mode (ssa_name, &unsignedp);
-	      always_initialized_rtx
-		= always_initialized_rtx_for_ssa_name_p (ssa_name);
-	    }
-
+	    pmode = promote_ssa_mode (ssa_name, &unsignedp);
 	  gcc_assert (GET_MODE (decl_rtl) == pmode);
 
 	  temp = gen_lowpart_SUBREG (mode, decl_rtl);
-
-	  /* We cannot assume anything about an existing extension if the
-	     register may contain uninitialized bits.  */
-	  if (always_initialized_rtx)
-	    {
-	      SUBREG_PROMOTED_VAR_P (temp) = 1;
-	      SUBREG_PROMOTED_SET (temp, unsignedp);
-	    }
-
+	  SUBREG_PROMOTED_VAR_P (temp) = 1;
+	  SUBREG_PROMOTED_SET (temp, unsignedp);
 	  return temp;
 	}
 
Index: loop-iv.c
===================================================================
--- loop-iv.c	(revision 253968)
+++ loop-iv.c	(working copy)
@@ -353,7 +353,7 @@  iv_get_reaching_def (rtx_insn *insn, rtx
   adef = DF_REF_CHAIN (use)->ref;
 
   /* We do not handle setting only part of the register.  */
-  if (DF_REF_FLAGS (adef) & (DF_REF_READ_WRITE | DF_REF_SUBREG))
+  if (DF_REF_FLAGS (adef) & DF_REF_READ_WRITE)
     return GRD_INVALID;
 
   def_insn = DF_REF_INSN (adef);
Index: tree-outof-ssa.h
===================================================================
--- tree-outof-ssa.h	(revision 253968)
+++ tree-outof-ssa.h	(working copy)
@@ -74,18 +74,6 @@  get_gimple_for_ssa_name (tree exp)
   return NULL;
 }
 
-/* Return whether the RTX expression representing the storage of the outof-SSA
-   partition that the SSA name EXP is a member of is always initialized.  */
-static inline bool
-always_initialized_rtx_for_ssa_name_p (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 !bitmap_bit_p (SA.partitions_for_undefined_values, p);
-}
-
 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);