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]

[RFC] Remove PUSH_ARGS_REVERSED from the RTL expander.


Hi,

Consider this testcase:

  extern void foo (int a, int b, int c, int d);

  void
  bar (int b, int c, int d)
  {
      foo (3, b, c, d);
  }

For many ABI's we will have on entry to the function

  r0 = b
  r1 = c
  r2 = d

If we process arguments to the call to foo left-to-right
(PUSH_ARGS_REVERSED == 0) we assign temporaries, and then hard registers
in this order:

  t0 = 3
  t1 = r0
  t2 = r1
  t3 = r2

  r0 = t0
  r1 = t1
  r2 = t2
  r3 = t3

We can't eliminate any of these temporaries as their live ranges overlap.

However, If we process the arguments right-to-left (PUSH_ARGS_REVERSED == 1),
we get this order:

  t0 = 3
  t1 = r0
  t2 = r1
  t3 = r2

  r3 = t3
  r2 = t2
  r1 = t1
  r0 = t0

And then we can start eliminating temporaries we don't need and get:

  r3 = r2
  r2 = r1
  r1 = r0
  r0 = 3

Which is much neater.

It seems to me that this would probably be of benefit on all targets.

This would be an argument for setting PUSH_ARGS_REVERSED to 1 by default
for all targets. However, this would have a perceivable impact on argument
evaluation order (which is unspecified in C, but people have shown
sensitivity to it changing in the past and we suspect some people may
have built systems relying on the behviour... ho hum...).

However, now that all side effects are handled when we are in SSA
form, it should be possible to refactor calls.c and expr.c as if
PUSH_ARGS_REVERSED were always defined to 1, without changing the
argument evaluation order in gimplify.c.

In this way, we have the best of both worlds; we maintain argument
evaluation order and gain the optimizations given by removing
overlapping live ranges for parameters.

I've bootstrapped and regression tested this on x86, ARM and AArch64 - but
I am well aware these are fairly standard targets, do you have any
suggestions or comments on which targets this change will affect?

If not, is this OK for stage-1?

Thanks,
James

---
gcc/

2014-03-21  James Greenhalgh  <james.greenhalgh@arm.com>

	* calls.c (initialize_argument_information): Always treat
	PUSH_ARGS_REVERSED as 1, simplify code accordingly.
	(expand_call): Likewise.
	(emit_library_call_calue_1): Likewise.
	* expr.c (PUSH_ARGS_REVERSED): Do not define.
	(emit_push_insn): Always treat PUSH_ARGS_REVERSED as 1, simplify
	code accordingly.
diff --git a/gcc/calls.c b/gcc/calls.c
index f392319..2e91918 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1104,8 +1104,6 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
 {
   CUMULATIVE_ARGS *args_so_far_pnt = get_cumulative_args (args_so_far);
   location_t loc = EXPR_LOCATION (exp);
-  /* 1 if scanning parms front to back, -1 if scanning back to front.  */
-  int inc;
 
   /* Count arg position in order args appear.  */
   int argpos;
@@ -1116,22 +1114,9 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
   args_size->var = 0;
 
   /* In this loop, we consider args in the order they are written.
-     We fill up ARGS from the front or from the back if necessary
-     so that in any case the first arg to be pushed ends up at the front.  */
+     We fill up ARGS from the back.  */
 
-  if (PUSH_ARGS_REVERSED)
-    {
-      i = num_actuals - 1, inc = -1;
-      /* In this case, must reverse order of args
-	 so that we compute and push the last arg first.  */
-    }
-  else
-    {
-      i = 0, inc = 1;
-    }
-
-  /* First fill in the actual arguments in the ARGS array, splitting
-     complex arguments if necessary.  */
+  i = num_actuals - 1;
   {
     int j = i;
     call_expr_arg_iterator iter;
@@ -1140,7 +1125,7 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
     if (struct_value_addr_value)
       {
 	args[j].tree_value = struct_value_addr_value;
-	j += inc;
+	j--;
       }
     FOR_EACH_CALL_EXPR_ARG (arg, iter, exp)
       {
@@ -1152,17 +1137,17 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
 	  {
 	    tree subtype = TREE_TYPE (argtype);
 	    args[j].tree_value = build1 (REALPART_EXPR, subtype, arg);
-	    j += inc;
+	    j--;
 	    args[j].tree_value = build1 (IMAGPART_EXPR, subtype, arg);
 	  }
 	else
 	  args[j].tree_value = arg;
-	j += inc;
+	j--;
       }
   }
 
   /* I counts args in order (to be) pushed; ARGPOS counts in order written.  */
-  for (argpos = 0; argpos < num_actuals; i += inc, argpos++)
+  for (argpos = 0; argpos < num_actuals; i--, argpos++)
     {
       tree type = TREE_TYPE (args[i].tree_value);
       int unsignedp;
@@ -2952,9 +2937,8 @@ expand_call (tree exp, rtx target, int ignore)
 
       compute_argument_addresses (args, argblock, num_actuals);
 
-      /* If we push args individually in reverse order, perform stack alignment
-	 before the first push (the last arg).  */
-      if (PUSH_ARGS_REVERSED && argblock == 0
+      /* Perform stack alignment before the first push (the last arg).  */
+      if (argblock == 0
           && adjusted_args_size.constant > reg_parm_stack_space
 	  && adjusted_args_size.constant != unadjusted_args_size)
 	{
@@ -3097,12 +3081,6 @@ expand_call (tree exp, rtx target, int ignore)
 		sibcall_failure = 1;
 	    }
 
-      /* If we pushed args in forward order, perform stack alignment
-	 after pushing the last arg.  */
-      if (!PUSH_ARGS_REVERSED && argblock == 0)
-	anti_adjust_stack (GEN_INT (adjusted_args_size.constant
-				    - unadjusted_args_size));
-
       /* If register arguments require space on the stack and stack space
 	 was not preallocated, allocate stack space here for arguments
 	 passed in registers.  */
@@ -3152,8 +3130,7 @@ expand_call (tree exp, rtx target, int ignore)
       if (pass == 1 && (return_flags & ERF_RETURNS_ARG))
 	{
 	  int arg_nr = return_flags & ERF_RETURN_ARG_MASK;
-	  if (PUSH_ARGS_REVERSED)
-	    arg_nr = num_actuals - arg_nr - 1;
+	  arg_nr = num_actuals - arg_nr - 1;
 	  if (arg_nr >= 0
 	      && arg_nr < num_actuals
 	      && args[arg_nr].reg
@@ -3597,7 +3574,6 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
      isn't present here, so we default to native calling abi here.  */
   tree fndecl ATTRIBUTE_UNUSED = NULL_TREE; /* library calls default to host calling abi ? */
   tree fntype ATTRIBUTE_UNUSED = NULL_TREE; /* library calls default to host calling abi ? */
-  int inc;
   int count;
   rtx argblock = 0;
   CUMULATIVE_ARGS args_so_far_v;
@@ -3946,22 +3922,13 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
 	argblock = push_block (GEN_INT (args_size.constant), 0, 0);
     }
 
-  /* If we push args individually in reverse order, perform stack alignment
+  /* We push args individually in reverse order, perform stack alignment
      before the first push (the last arg).  */
-  if (argblock == 0 && PUSH_ARGS_REVERSED)
+  if (argblock == 0)
     anti_adjust_stack (GEN_INT (args_size.constant
 				- original_args_size.constant));
 
-  if (PUSH_ARGS_REVERSED)
-    {
-      inc = -1;
-      argnum = nargs - 1;
-    }
-  else
-    {
-      inc = 1;
-      argnum = 0;
-    }
+  argnum = nargs - 1;
 
 #ifdef REG_PARM_STACK_SPACE
   if (ACCUMULATE_OUTGOING_ARGS)
@@ -3978,7 +3945,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
 
   /* ARGNUM indexes the ARGVEC array in the order in which the arguments
      are to be pushed.  */
-  for (count = 0; count < nargs; count++, argnum += inc)
+  for (count = 0; count < nargs; count++, argnum--)
     {
       enum machine_mode mode = argvec[argnum].mode;
       rtx val = argvec[argnum].value;
@@ -4080,16 +4047,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
 	}
     }
 
-  /* If we pushed args in forward order, perform stack alignment
-     after pushing the last arg.  */
-  if (argblock == 0 && !PUSH_ARGS_REVERSED)
-    anti_adjust_stack (GEN_INT (args_size.constant
-				- original_args_size.constant));
-
-  if (PUSH_ARGS_REVERSED)
-    argnum = nargs - 1;
-  else
-    argnum = 0;
+  argnum = nargs - 1;
 
   fun = prepare_call_address (NULL, fun, NULL, &call_fusage, 0, 0);
 
@@ -4097,7 +4055,7 @@ emit_library_call_value_1 (int retval, rtx orgfun, rtx value,
 
   /* ARGNUM indexes the ARGVEC array in the order in which the arguments
      are to be pushed.  */
-  for (count = 0; count < nargs; count++, argnum += inc)
+  for (count = 0; count < nargs; count++, argnum--)
     {
       enum machine_mode mode = argvec[argnum].mode;
       rtx val = argvec[argnum].value;
diff --git a/gcc/expr.c b/gcc/expr.c
index be62c53..2ef6c63 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -68,22 +68,6 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-ssa-address.h"
 #include "cfgexpand.h"
 
-/* Decide whether a function's arguments should be processed
-   from first to last or from last to first.
-
-   They should if the stack and args grow in opposite directions, but
-   only if we have push insns.  */
-
-#ifdef PUSH_ROUNDING
-
-#ifndef PUSH_ARGS_REVERSED
-#if defined (STACK_GROWS_DOWNWARD) != defined (ARGS_GROW_DOWNWARD)
-#define PUSH_ARGS_REVERSED	/* If it's last to first.  */
-#endif
-#endif
-
-#endif
-
 #ifndef STACK_PUSH_CODE
 #ifdef STACK_GROWS_DOWNWARD
 #define STACK_PUSH_CODE PRE_DEC
@@ -4354,11 +4338,7 @@ emit_push_insn (rtx x, enum machine_mode mode, tree type, rtx size,
       /* Loop over all the words allocated on the stack for this arg.  */
       /* We can do it by words, because any scalar bigger than a word
 	 has a size a multiple of a word.  */
-#ifndef PUSH_ARGS_REVERSED
-      for (i = not_stack; i < size; i++)
-#else
       for (i = size - 1; i >= not_stack; i--)
-#endif
 	if (i >= not_stack + offset)
 	  emit_push_insn (operand_subword_force (x, i, mode),
 			  word_mode, NULL_TREE, NULL_RTX, align, 0, NULL_RTX,

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