diff mbox

Perform some kind of special TER for asm input operands that don't allow regs nor memory (PR inline-asm/23200)

Message ID 20110203182147.GY30899@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Feb. 3, 2011, 6:21 p.m. UTC 
Hi!

The attached testcase fails to assemble at -O0, because at -O0
we don't perform TER (and anyway, TER for -O0 wants to ensure the
locus stays the same, while in the testcase there are different columns
involved).  The following patch, instead of enabling TER at -O0,
instead just only looks at STMTs defining replaceable SSA_NAMEs
when expanding with EXPAND_INITIALIZER (which is where we usually want
something constant, among most important places asm input operands
that allow neither regs nor mems).

Perhaps we want to limit it some more, e.g. don't look at SSA_NAME_DEF_STMT
if it sets a SSA_NAME for user variable at -O0, and perhaps expr.c
should perform this only if (!optimize || !flag_tree_ter).
If we'd avoid user variables (!DECL_IGNORED_P or something similar), then
pr27528.c testcase wouldn't need to be adjusted.

Bootstrapped/regtested on x86_64-linux and i686-linux.

2011-02-03  Jakub Jelinek  <jakub@redhat.com>

	PR inline-asm/23200
	* tree-ssa-ter.c (is_replaceable_p): Add TER argument.  Don't
	do bb, locus and block comparison and disallow loads if it is
	not set.
	(stmt_is_replaceable_p): New function.
	(process_replaceable, find_replaceable_in_bb): Adjust is_replaceable_p
	callers.
	* expr.c (expand_expr_real_1) <case SSA_NAME>: If
	get_gimple_for_ssa_name try for EXPAND_INITIALIZER harder to use
	SSA_NAME_DEF_STMT.
	* tree-flow.h (stmt_is_replaceable_p): New prototype.

	* gcc.dg/pr23200.c: New test.
	* gcc.dg/pr27528.c: Remove one expected error and warning.


	Jakub

Comments

Richard Henderson Feb. 3, 2011, 7:22 p.m. UTC | #1 
On 02/03/2011 10:21 AM, Jakub Jelinek wrote:
> Hi!
> 
> The attached testcase fails to assemble at -O0, because at -O0
> we don't perform TER (and anyway, TER for -O0 wants to ensure the
> locus stays the same, while in the testcase there are different columns
> involved).  The following patch, instead of enabling TER at -O0,
> instead just only looks at STMTs defining replaceable SSA_NAMEs
> when expanding with EXPAND_INITIALIZER (which is where we usually want
> something constant, among most important places asm input operands
> that allow neither regs nor mems).
> 
> Perhaps we want to limit it some more, e.g. don't look at SSA_NAME_DEF_STMT
> if it sets a SSA_NAME for user variable at -O0, and perhaps expr.c
> should perform this only if (!optimize || !flag_tree_ter).
> If we'd avoid user variables (!DECL_IGNORED_P or something similar), then
> pr27528.c testcase wouldn't need to be adjusted.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux.
> 
> 2011-02-03  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR inline-asm/23200
> 	* tree-ssa-ter.c (is_replaceable_p): Add TER argument.  Don't
> 	do bb, locus and block comparison and disallow loads if it is
> 	not set.
> 	(stmt_is_replaceable_p): New function.
> 	(process_replaceable, find_replaceable_in_bb): Adjust is_replaceable_p
> 	callers.
> 	* expr.c (expand_expr_real_1) <case SSA_NAME>: If
> 	get_gimple_for_ssa_name try for EXPAND_INITIALIZER harder to use
> 	SSA_NAME_DEF_STMT.

The expand_expr change seems fine.

The is_replaceable_p change lacks documentation for the TER argument,
and reading the patch I wasn't sure precisely what it is supposed to mean.


r~
Jakub Jelinek Feb. 3, 2011, 8:14 p.m. UTC | #2 
On Thu, Feb 03, 2011 at 11:22:49AM -0800, Richard Henderson wrote:
> On 02/03/2011 10:21 AM, Jakub Jelinek wrote:
> The is_replaceable_p change lacks documentation for the TER argument,
> and reading the patch I wasn't sure precisely what it is supposed to mean.

I can certainly document it.

The meaning of the argument was that TER is true when used in ter pass,
and false when called from EXPAND_INITIALIZER expansion.  In the latter
case I thought that we don't mind different locuses for -O0 (after all,
the initializer is not something people will be able to step through
anyway), don't mind even stmts from different BBs and different BLOCKs,
but don't want any memory loads (those wouldn't make a constant anyway).

	Jakub
diff mbox

Patch

--- gcc/tree-ssa-ter.c.jj	2010-12-17 10:03:49.000000000 +0100
+++ gcc/tree-ssa-ter.c	2011-02-03 14:05:32.000000000 +0100
@@ -360,7 +360,7 @@  add_dependence (temp_expr_table_p tab, i
 /* Return TRUE if expression STMT is suitable for replacement.  */
 
 static inline bool
-is_replaceable_p (gimple stmt)
+is_replaceable_p (gimple stmt, bool ter)
 {
   use_operand_p use_p;
   tree def;
@@ -386,7 +386,7 @@  is_replaceable_p (gimple stmt)
     return false;
 
   /* If the use isn't in this block, it wont be replaced either.  */
-  if (gimple_bb (use_stmt) != gimple_bb (stmt))
+  if (ter && gimple_bb (use_stmt) != gimple_bb (stmt))
     return false;
 
   locus1 = gimple_location (stmt);
@@ -404,6 +404,7 @@  is_replaceable_p (gimple stmt)
     }
 
   if (!optimize
+      && ter
       && ((locus1 && locus1 != locus2) || (block1 && block1 != block2)))
     return false;
 
@@ -416,7 +417,7 @@  is_replaceable_p (gimple stmt)
     return false;
 
   /* Without alias info we can't move around loads.  */
-  if (!optimize
+  if ((!optimize || !ter)
       && gimple_assign_single_p (stmt)
       && !is_gimple_val (gimple_assign_rhs1 (stmt)))
     return false;
@@ -444,6 +445,16 @@  is_replaceable_p (gimple stmt)
 }
 
 
+/* 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.  */
@@ -477,7 +488,7 @@  process_replaceable (temp_expr_table_p t
   ssa_op_iter iter;
   bitmap def_vars, use_vars;
 
-  gcc_checking_assert (is_replaceable_p (stmt));
+  gcc_checking_assert (is_replaceable_p (stmt, true));
 
   def = SINGLE_SSA_TREE_OPERAND (stmt, SSA_OP_DEF);
   version = SSA_NAME_VERSION (def);
@@ -589,7 +600,7 @@  find_replaceable_in_bb (temp_expr_table_
       if (is_gimple_debug (stmt))
 	continue;
 
-      stmt_replaceable = is_replaceable_p (stmt);
+      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)
--- gcc/expr.c.jj	2011-02-03 11:37:02.000000000 +0100
+++ gcc/expr.c	2011-02-03 14:12:03.000000000 +0100
@@ -8387,6 +8387,12 @@  expand_expr_real_1 (tree exp, rtx target
 				   NULL);
 
       g = get_gimple_for_ssa_name (exp);
+      /* For EXPAND_INITIALIZER try harder to get something simpler.  */
+      if (g == NULL
+	  && modifier == EXPAND_INITIALIZER
+	  && !SSA_NAME_IS_DEFAULT_DEF (exp)
+	  && stmt_is_replaceable_p (SSA_NAME_DEF_STMT (exp)))
+	g = SSA_NAME_DEF_STMT (exp);
       if (g)
 	return expand_expr_real (gimple_assign_rhs_to_tree (g), target, tmode,
 				 modifier, NULL);
--- gcc/tree-flow.h.jj	2011-01-31 14:11:31.000000000 +0100
+++ gcc/tree-flow.h	2011-01-31 14:11:31.000000000 +0100
@@ -852,6 +852,9 @@  bool fixup_noreturn_call (gimple stmt);
 /* In ipa-pure-const.c  */
 void warn_function_noreturn (tree);
 
+/* In tree-ssa-ter.c  */
+bool stmt_is_replaceable_p (gimple);
+
 #include "tree-flow-inline.h"
 
 void swap_tree_operands (gimple, tree *, tree *);
--- gcc/testsuite/gcc.dg/pr23200.c.jj	2011-02-03 16:07:16.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr23200.c	2011-02-03 16:07:06.000000000 +0100
@@ -0,0 +1,22 @@ 
+/* PR inline-asm/23200 */
+/* { dg-do compile { target nonpic } } */
+/* { dg-options "-O0" } */
+
+static char var;
+
+void
+foo (void)
+{
+  asm volatile ("" :: "i" (&var + 1));
+}
+
+typedef int T[];
+typedef T *P;
+
+int var2;
+
+void
+bar (void)
+{
+  asm volatile ("" :: "i"(&(*(P)&var2)[1]));
+}
--- gcc/testsuite/gcc.dg/pr27528.c.jj	2010-06-11 11:00:48.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr27528.c	2011-02-03 18:33:09.153670962 +0100
@@ -4,7 +4,7 @@ 
 /* { dg-error "impossible constraint" "" { target *-*-* } 13 } */
 /* { dg-error "impossible constraint" "" { target *-*-* } 14 } */
 /* { dg-error "impossible constraint" "" { target *-*-* } 15 } */
-/* { dg-error "impossible constraint" "" { target *-*-* } 16 } */
+
 int bar (int);
 void
 foo (int *x, int y)
@@ -13,6 +13,6 @@  foo (int *x, int y)
   asm ("# %0" :: "i" (x)); /* { dg-warning "probably doesn't match" } */
   asm ("# %0" :: "i" (bar (*x))); /* { dg-warning "probably doesn't match" } */
   asm ("# %0" :: "i" (*x + 0x11)); /* { dg-warning "probably doesn't match" } */
-  asm ("# %0" :: "i" (constant)); /* { dg-warning "probably doesn't match" } */
+  asm ("# %0" :: "i" (constant)); /* optimized */
   asm ("# %0" :: "i" (y * 0)); /* folded */
 }