Re: [PATCH 2/2] [RS6000] REG_PARM_STACK_SPACE check V2

2020-11-02 Thread Segher Boessenkool
On Mon, Nov 02, 2020 at 08:19:08PM +1030, Alan Modra wrote:
> On PowerPC we can tail call if the callee has less or equal
> REG_PARM_STACK_SPACE than the caller, as demonstrated by the
> testcase.  So we should use
> 
>   /* If reg parm stack space increases, we cannot sibcall.  */
>   if (REG_PARM_STACK_SPACE (decl ? decl : fntype)
>   > INCOMING_REG_PARM_STACK_SPACE (current_function_decl))
> 
> and note the change to use INCOMING_REG_PARM_STACK_SPACE.

> Bootstrapped and regression tested powerpc64le-linux and biarch
> powerpc64-linux. OK?

Yes please.  Thanks!


Segher


>   PR middle-end/97267
> gcc/
>   * config/rs6000/rs6000-logue.c (rs6000_function_ok_for_sibcall):
>   Remove code checking REG_PARM_STACK_SPACE.
> testsuite/
>   * gcc.target/powerpc/pr97267.c: New test.


[PATCH 2/2] [RS6000] REG_PARM_STACK_SPACE check V2

2020-11-02 Thread Alan Modra via Gcc-patches
On PowerPC we can tail call if the callee has less or equal
REG_PARM_STACK_SPACE than the caller, as demonstrated by the
testcase.  So we should use

  /* If reg parm stack space increases, we cannot sibcall.  */
  if (REG_PARM_STACK_SPACE (decl ? decl : fntype)
  > INCOMING_REG_PARM_STACK_SPACE (current_function_decl))

and note the change to use INCOMING_REG_PARM_STACK_SPACE.
REG_PARM_STACK_SPACE has always been wrong there for PowerPC.  See
https://gcc.gnu.org/pipermail/gcc-patches/2014-May/389867.html for why
if you're curious.  Not that it matters, because PowerPC can do
without this check entirely, relying on a stack slot test in generic
code.

a) The generic code checks that arg passing stack in the callee is not
   greater than that in the caller, and,
b) ELFv2 only allocates reg_parm_stack_space when some parameter is
   passed on the stack.
Point (b) means that zero reg_parm_stack_space implies zero stack
space, and non-zero reg_parm_stack_space implies non-zero stack
space.  So the case of 0 reg_parm_stack_space in the caller and 64 in
the callee will be caught by (a).

Bootstrapped and regression tested powerpc64le-linux and biarch
powerpc64-linux. OK?

PR middle-end/97267
gcc/
* config/rs6000/rs6000-logue.c (rs6000_function_ok_for_sibcall):
Remove code checking REG_PARM_STACK_SPACE.
testsuite/
* gcc.target/powerpc/pr97267.c: New test.

diff --git a/gcc/config/rs6000/rs6000-logue.c b/gcc/config/rs6000/rs6000-logue.c
index 61eb7ce7ade..d90cd5736e1 100644
--- a/gcc/config/rs6000/rs6000-logue.c
+++ b/gcc/config/rs6000/rs6000-logue.c
@@ -30,7 +30,6 @@
 #include "df.h"
 #include "tm_p.h"
 #include "ira.h"
-#include "calls.h"
 #include "print-tree.h"
 #include "varasm.h"
 #include "explow.h"
@@ -1134,19 +1133,6 @@ rs6000_function_ok_for_sibcall (tree decl, tree exp)
   else
 fntype = TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (exp)));
 
-  /* If outgoing reg parm stack space changes, we cannot do sibcall.  */
-  if ((OUTGOING_REG_PARM_STACK_SPACE (fntype)
-   != OUTGOING_REG_PARM_STACK_SPACE (TREE_TYPE (current_function_decl)))
-  || (REG_PARM_STACK_SPACE (decl ? decl : fntype)
- != REG_PARM_STACK_SPACE (current_function_decl)))
-{
-  maybe_complain_about_tail_call (exp,
- "inconsistent size of stack space"
- " allocated for arguments which are"
- " passed in registers");
-  return false;
-}
-
   /* We can't do it if the called function has more vector parameters
  than the current function; there's nowhere to put the VRsave code.  */
   if (TARGET_ALTIVEC_ABI
diff --git a/gcc/testsuite/gcc.target/powerpc/pr97267.c 
b/gcc/testsuite/gcc.target/powerpc/pr97267.c
new file mode 100644
index 000..cab46245fc9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr97267.c
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+static int __attribute__ ((__noclone__, __noinline__))
+reg_args (int j1, int j2, int j3, int j4, int j5, int j6, int j7, int j8)
+{
+  return j1 + j2 + j3 + j4 + j5 + j6 + j7 + j8;
+}
+
+int __attribute__ ((__noclone__, __noinline__))
+stack_args (int j1, int j2, int j3, int j4, int j5, int j6, int j7, int j8,
+   int j9)
+{
+  if (j9 == 0)
+return 0;
+  return reg_args (j1, j2, j3, j4, j5, j6, j7, j8);
+}
+
+/* { dg-final { scan-assembler-not {(?n)^\s+bl\s} } } */

-- 
Alan Modra
Australia Development Lab, IBM