[MIPS] Implement static stack checking

[MIPS] Implement static stack checking

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]

This implements static stack checking for MIPS, i.e. checking of the static 
part of the frame in the prologue when -fstack-check is specified.  This is 
very similar to the PowerPC and SPARC implementations and makes it possible to 
pass the full ACATS testsuite with -fstack-check.

Tested on mips64el-linux-gnu (n32/32/64), OK for the mainline?


2012-10-21  Eric Botcazou  <ebotcazou@adacore.com>

	* config/mips/linux-common.h (STACK_CHECK_STATIC_BUILTIN): Define.
	(STACK_CHECK_PROTECT): Likewise.
	* config/mips/mips-protos.h (mips_output_probe_stack_range): Declare.
	* config/mips/mips.c: Include common/common-target.h.
	(mips_emit_probe_stack_range): New function.
	(mips_output_probe_stack_range): Likewise.
	(mips_expand_prologue): Invoke mips_emit_probe_stack_range if static
	builtin stack checking is enabled.
	* config/mips/mips.md (UNSPEC_PROBE_STACK_RANGE): New constant.
	(probe_stack_range<P:mode>): New insn.
ada/
	* system-linux-mipsel.ads (Stack_Check_Probes): Set to True.
	* system-linux-mips.ads (Stack_Check_Probes): Likewise.
	* system-linux-mips64el.ads (Stack_Check_Probes): Likewise.


-- 
Eric Botcazou

[-- Attachment #2: mips_stack_check.diff --]
[-- Type: text/x-patch, Size: 11633 bytes --]

Index: config/mips/mips.md
===================================================================
--- config/mips/mips.md	(revision 192648)
+++ config/mips/mips.md	(working copy)
@@ -137,6 +137,9 @@ (define_c_enum "unspec" [
 
   ;; MIPS16 casesi jump table dispatch.
   UNSPEC_CASESI_DISPATCH
+
+  ;; Stack checking.
+  UNSPEC_PROBE_STACK_RANGE
 ])
 
 (define_constants
@@ -6040,6 +6043,17 @@ (define_insn "blockage"
   [(set_attr "type" "ghost")
    (set_attr "mode" "none")])
 
+(define_insn "probe_stack_range<P:mode>"
+  [(set (match_operand:P 0 "register_operand" "=r")
+	(unspec_volatile:P [(match_operand:P 1 "register_operand" "0")
+			    (match_operand:P 2 "register_operand" "r")]
+			    UNSPEC_PROBE_STACK_RANGE))]
+  ""
+  "* return mips_output_probe_stack_range (operands[0], operands[2]);"
+  [(set_attr "type" "unknown")
+   (set_attr "can_delay" "no")
+   (set_attr "mode" "<MODE>")])
+
 (define_expand "epilogue"
   [(const_int 2)]
   ""
Index: config/mips/linux-common.h
===================================================================
--- config/mips/linux-common.h	(revision 192648)
+++ config/mips/linux-common.h	(working copy)
@@ -56,3 +56,9 @@ along with GCC; see the file COPYING3.
 		       GNU_USER_TARGET_ENDFILE_SPEC,			\
 		       GNU_USER_TARGET_MATHFILE_SPEC " "		\
 		       ANDROID_ENDFILE_SPEC)
+
+/* Define this to be nonzero if static stack checking is supported.  */
+#define STACK_CHECK_STATIC_BUILTIN 1
+
+/* The default value isn't sufficient in 64-bit mode.  */
+#define STACK_CHECK_PROTECT (TARGET_64BIT ? 16 * 1024 : 12 * 1024)
Index: config/mips/mips-protos.h
===================================================================
--- config/mips/mips-protos.h	(revision 192648)
+++ config/mips/mips-protos.h	(working copy)
@@ -316,6 +316,7 @@ extern const char *mips_output_sync (voi
 extern const char *mips_output_sync_loop (rtx, rtx *);
 extern unsigned int mips_sync_loop_insns (rtx, rtx *);
 extern const char *mips_output_division (const char *, rtx *);
+extern const char *mips_output_probe_stack_range (rtx, rtx);
 extern unsigned int mips_hard_regno_nregs (int, enum machine_mode);
 extern bool mips_linked_madd_p (rtx, rtx);
 extern bool mips_store_data_bypass_p (rtx, rtx);
Index: config/mips/mips.c
===================================================================
--- config/mips/mips.c	(revision 192648)
+++ config/mips/mips.c	(working copy)
@@ -50,6 +50,7 @@ along with GCC; see the file COPYING3.
 #include "debug.h"
 #include "target.h"
 #include "target-def.h"
+#include "common/common-target.h"
 #include "langhooks.h"
 #include "sched-int.h"
 #include "gimple.h"
@@ -10613,6 +10614,144 @@ mips_emit_loadgp (void)
     emit_insn (gen_loadgp_blockage ());
 }
 
+#define PROBE_INTERVAL (1 << STACK_CHECK_PROBE_INTERVAL_EXP)
+
+#if PROBE_INTERVAL > 32768
+#error Cannot use indexed addressing mode for stack probing
+#endif
+
+/* Emit code to probe a range of stack addresses from FIRST to FIRST+SIZE,
+   inclusive.  These are offsets from the current stack pointer.  */
+
+static void
+mips_emit_probe_stack_range (HOST_WIDE_INT first, HOST_WIDE_INT size)
+{
+  /* See if we have a constant small number of probes to generate.  If so,
+     that's the easy case.  */
+  if (first + size <= 32768)
+    {
+      HOST_WIDE_INT i;
+
+      /* Probe at FIRST + N * PROBE_INTERVAL for values of N from 1 until
+	 it exceeds SIZE.  If only one probe is needed, this will not
+	 generate any code.  Then probe at FIRST + SIZE.  */
+      for (i = PROBE_INTERVAL; i < size; i += PROBE_INTERVAL)
+        emit_stack_probe (plus_constant (Pmode, stack_pointer_rtx,
+					 -(first + i)));
+
+      emit_stack_probe (plus_constant (Pmode, stack_pointer_rtx,
+				       -(first + size)));
+    }
+
+  /* Otherwise, do the same as above, but in a loop.  Note that we must be
+     extra careful with variables wrapping around because we might be at
+     the very top (or the very bottom) of the address space and we have
+     to be able to handle this case properly; in particular, we use an
+     equality test for the loop condition.  */
+  else
+    {
+      HOST_WIDE_INT rounded_size;
+      rtx r3 = gen_rtx_REG (Pmode, GP_REG_FIRST + 3);
+      rtx r12 = gen_rtx_REG (Pmode, GP_REG_FIRST + 12);
+
+      /* Sanity check for the addressing mode we're going to use.  */
+      gcc_assert (first <= 32768);
+
+
+      /* Step 1: round SIZE to the previous multiple of the interval.  */
+
+      rounded_size = size & -PROBE_INTERVAL;
+
+
+      /* Step 2: compute initial and final value of the loop counter.  */
+
+      /* TEST_ADDR = SP + FIRST.  */
+      emit_insn (gen_rtx_SET (VOIDmode, r3,
+			      plus_constant (Pmode, stack_pointer_rtx,
+					     -first)));
+
+      /* LAST_ADDR = SP + FIRST + ROUNDED_SIZE.  */
+      if (rounded_size > 32768)
+	{
+          emit_move_insn (r12, GEN_INT (rounded_size));
+	  emit_insn (gen_rtx_SET (VOIDmode, r12,
+			          gen_rtx_MINUS (Pmode, r3, r12)));
+	}
+      else
+	emit_insn (gen_rtx_SET (VOIDmode, r12,
+			        plus_constant (Pmode, r3, -rounded_size)));
+
+
+      /* Step 3: the loop
+
+	while (TEST_ADDR != LAST_ADDR)
+	  {
+	    TEST_ADDR = TEST_ADDR + PROBE_INTERVAL
+	    probe at TEST_ADDR
+	  }
+
+	probes at FIRST + N * PROBE_INTERVAL for values of N from 1
+	until it is equal to ROUNDED_SIZE.  */
+
+      if (TARGET_64BIT && TARGET_LONG64)
+	emit_insn (gen_probe_stack_rangedi (r3, r3, r12));
+      else
+	emit_insn (gen_probe_stack_rangesi (r3, r3, r12));
+
+
+      /* Step 4: probe at FIRST + SIZE if we cannot assert at compile-time
+	 that SIZE is equal to ROUNDED_SIZE.  */
+
+      if (size != rounded_size)
+	emit_stack_probe (plus_constant (Pmode, r12, rounded_size - size));
+    }
+
+  /* Make sure nothing is scheduled before we are done.  */
+  emit_insn (gen_blockage ());
+}
+
+/* Probe a range of stack addresses from REG1 to REG2 inclusive.  These are
+   absolute addresses.  */
+
+const char *
+mips_output_probe_stack_range (rtx reg1, rtx reg2)
+{
+  static int labelno = 0;
+  char loop_lab[32], end_lab[32], tmp[64];
+  rtx xops[2];
+
+  ASM_GENERATE_INTERNAL_LABEL (loop_lab, "LPSRL", labelno);
+  ASM_GENERATE_INTERNAL_LABEL (end_lab, "LPSRE", labelno++);
+
+  ASM_OUTPUT_INTERNAL_LABEL (asm_out_file, loop_lab);
+
+  /* Jump to END_LAB if TEST_ADDR == LAST_ADDR.  */
+  xops[0] = reg1;
+  xops[1] = reg2;
+  strcpy (tmp, "beq\t%0,%1,");
+  output_asm_insn (strcat (tmp, &end_lab[1]), xops);
+ 
+  /* TEST_ADDR = TEST_ADDR + PROBE_INTERVAL.  */
+  xops[1] = GEN_INT (-PROBE_INTERVAL);
+  if (TARGET_64BIT && TARGET_LONG64)
+    output_asm_insn (" daddiu\t%0,%0,%1", xops);
+  else
+    output_asm_insn (" addiu\t%0,%0,%1", xops);
+
+  /* Probe at TEST_ADDR and branch.  */
+  fprintf (asm_out_file, "\tb\t");
+  assemble_name_raw (asm_out_file, loop_lab);
+  fputc ('\n', asm_out_file);
+  if (TARGET_64BIT)
+    output_asm_insn (" sd\t$0,0(%0)", xops);
+  else
+    output_asm_insn (" sw\t$0,0(%0)", xops);
+
+  ASM_OUTPUT_INTERNAL_LABEL (asm_out_file, end_lab);
+
+  return "";
+}
+
 /* A for_each_rtx callback.  Stop the search if *X is a kernel register.  */
 
 static int
@@ -10652,6 +10791,9 @@ mips_expand_prologue (void)
   if (flag_stack_usage_info)
     current_function_static_stack_size = size;
 
+  if (flag_stack_check == STATIC_BUILTIN_STACK_CHECK && size)
+    mips_emit_probe_stack_range (STACK_CHECK_PROTECT, size);
+
   /* Save the registers.  Allocate up to MIPS_MAX_FIRST_STACK_STEP
      bytes beforehand; this is enough to cover the register save area
      without going out of range.  */
Index: ada/system-linux-mipsel.ads
===================================================================
--- ada/system-linux-mipsel.ads	(revision 192648)
+++ ada/system-linux-mipsel.ads	(working copy)
@@ -7,7 +7,7 @@
 --                                 S p e c                                  --
 --                        (GNU-Linux/MIPSEL Version)                        --
 --                                                                          --
---          Copyright (C) 1992-2011, Free Software Foundation, Inc.         --
+--          Copyright (C) 1992-2012, Free Software Foundation, Inc.         --
 --                                                                          --
 -- This specification is derived from the Ada Reference Manual for use with --
 -- GNAT. The copyright notice above, and the license provisions that follow --
@@ -128,7 +128,7 @@ private
    Preallocated_Stacks       : constant Boolean := False;
    Signed_Zeros              : constant Boolean := True;
    Stack_Check_Default       : constant Boolean := False;
-   Stack_Check_Probes        : constant Boolean := False;
+   Stack_Check_Probes        : constant Boolean := True;
    Stack_Check_Limits        : constant Boolean := False;
    Support_64_Bit_Divides    : constant Boolean := True;
    Support_Aggregates        : constant Boolean := True;
Index: ada/system-linux-mips.ads
===================================================================
--- ada/system-linux-mips.ads	(revision 192648)
+++ ada/system-linux-mips.ads	(working copy)
@@ -7,7 +7,7 @@
 --                                 S p e c                                  --
 --                          (GNU-Linux/MIPS Version)                        --
 --                                                                          --
---          Copyright (C) 1992-2011, Free Software Foundation, Inc.         --
+--          Copyright (C) 1992-2012, Free Software Foundation, Inc.         --
 --                                                                          --
 -- This specification is derived from the Ada Reference Manual for use with --
 -- GNAT. The copyright notice above, and the license provisions that follow --
@@ -128,7 +128,7 @@ private
    Preallocated_Stacks       : constant Boolean := False;
    Signed_Zeros              : constant Boolean := True;
    Stack_Check_Default       : constant Boolean := False;
-   Stack_Check_Probes        : constant Boolean := False;
+   Stack_Check_Probes        : constant Boolean := True;
    Stack_Check_Limits        : constant Boolean := False;
    Support_64_Bit_Divides    : constant Boolean := True;
    Support_Aggregates        : constant Boolean := True;
Index: ada/system-linux-mips64el.ads
===================================================================
--- ada/system-linux-mips64el.ads	(revision 192648)
+++ ada/system-linux-mips64el.ads	(working copy)
@@ -7,7 +7,7 @@
 --                                 S p e c                                  --
 --                       (GNU-Linux/MIPS64EL Version)                       --
 --                                                                          --
---          Copyright (C) 1992-2011, Free Software Foundation, Inc.         --
+--          Copyright (C) 1992-2012, Free Software Foundation, Inc.         --
 --                                                                          --
 -- This specification is derived from the Ada Reference Manual for use with --
 -- GNAT. The copyright notice above, and the license provisions that follow --
@@ -128,7 +128,7 @@ private
    Preallocated_Stacks       : constant Boolean := False;
    Signed_Zeros              : constant Boolean := True;
    Stack_Check_Default       : constant Boolean := False;
-   Stack_Check_Probes        : constant Boolean := False;
+   Stack_Check_Probes        : constant Boolean := True;
    Stack_Check_Limits        : constant Boolean := False;
    Support_64_Bit_Divides    : constant Boolean := True;
    Support_Aggregates        : constant Boolean := True;

Re: [MIPS] Implement static stack checking

[Richard Sandiford]

Eric Botcazou <ebotcazou@adacore.com> writes:
> This implements static stack checking for MIPS, i.e. checking of the static 
> part of the frame in the prologue when -fstack-check is specified.  This is 
> very similar to the PowerPC and SPARC implementations and makes it possible to 
> pass the full ACATS testsuite with -fstack-check.
>
> Tested on mips64el-linux-gnu (n32/32/64), OK for the mainline?

The Ada bits I'll leave to you. :-)  The config/mips stuff looks good,
but a couple of nits:

> +(define_insn "probe_stack_range<P:mode>"
> +  [(set (match_operand:P 0 "register_operand" "=r")
> +	(unspec_volatile:P [(match_operand:P 1 "register_operand" "0")
> +			    (match_operand:P 2 "register_operand" "r")]
> +			    UNSPEC_PROBE_STACK_RANGE))]
> +  ""
> +  "* return mips_output_probe_stack_range (operands[0], operands[2]);"
> +  [(set_attr "type" "unknown")
> +   (set_attr "can_delay" "no")
> +   (set_attr "mode" "<MODE>")])

Please use "d" rather than "r" in these constraints.  Please use:

  { return mips_output_probe_stack_range (operands[0], operands[2]); }

for the output line.

> +/* Emit code to probe a range of stack addresses from FIRST to FIRST+SIZE,
> +   inclusive.  These are offsets from the current stack pointer.  */
> +
> +static void
> +mips_emit_probe_stack_range (HOST_WIDE_INT first, HOST_WIDE_INT size)
> +{

This function doesn't work with MIPS16 mode.  Maybe just:

  if (TARGET_MIPS16)
    sorry ("MIPS16 stack probes");

(We can't test TARGET_MIPS16 in something like STACK_CHECK_STATIC_BUILTIN
because MIPS16ness is a per-function property.)

> +  /* See if we have a constant small number of probes to generate.  If so,
> +     that's the easy case.  */
> +  if (first + size <= 32768)
> +    {
> +      HOST_WIDE_INT i;
> +
> +      /* Probe at FIRST + N * PROBE_INTERVAL for values of N from 1 until
> +	 it exceeds SIZE.  If only one probe is needed, this will not
> +	 generate any code.  Then probe at FIRST + SIZE.  */
> +      for (i = PROBE_INTERVAL; i < size; i += PROBE_INTERVAL)
> +        emit_stack_probe (plus_constant (Pmode, stack_pointer_rtx,
> +					 -(first + i)));
> +
> +      emit_stack_probe (plus_constant (Pmode, stack_pointer_rtx,
> +				       -(first + size)));
> +    }
> +
> +  /* Otherwise, do the same as above, but in a loop.  Note that we must be
> +     extra careful with variables wrapping around because we might be at
> +     the very top (or the very bottom) of the address space and we have
> +     to be able to handle this case properly; in particular, we use an
> +     equality test for the loop condition.  */
> +  else
> +    {
> +      HOST_WIDE_INT rounded_size;
> +      rtx r3 = gen_rtx_REG (Pmode, GP_REG_FIRST + 3);
> +      rtx r12 = gen_rtx_REG (Pmode, GP_REG_FIRST + 12);

Please use MIPS_PROLOGUE_TEMP for r3 (and probably rename r3).
I suppose GP_REG_FIRST + 12 should be MIPS_PROLOGUE_TEMP2, probably as:

#define MIPS_PROLOGUE_TEMP2_REGNUM \
  (TARGET_MIPS16 ? gcc_unreachable () \
   cfun->machine->interrupt_handler_p ? K1_REG_NUM : GP_REG_FIRST + 12)

#define MIPS_PROLOGUE_TEMP2(MODE) \
  gen_rtx_REG (MODE, MIPS_PROLOGUE_TEMP2_REGNUM)

and update the block comment above the MIPS_PROLOGUE_TEMP_REGNUM definition.

> +      /* Sanity check for the addressing mode we're going to use.  */
> +      gcc_assert (first <= 32768);
> +
> +
> +      /* Step 1: round SIZE to the previous multiple of the interval.  */
> +
> +      rounded_size = size & -PROBE_INTERVAL;
> +
> +
> +      /* Step 2: compute initial and final value of the loop counter.  */
> +
> +      /* TEST_ADDR = SP + FIRST.  */
> +      emit_insn (gen_rtx_SET (VOIDmode, r3,
> +			      plus_constant (Pmode, stack_pointer_rtx,
> +					     -first)));
> +
> +      /* LAST_ADDR = SP + FIRST + ROUNDED_SIZE.  */
> +      if (rounded_size > 32768)
> +	{
> +          emit_move_insn (r12, GEN_INT (rounded_size));
> +	  emit_insn (gen_rtx_SET (VOIDmode, r12,
> +			          gen_rtx_MINUS (Pmode, r3, r12)));
> +	}
> +      else
> +	emit_insn (gen_rtx_SET (VOIDmode, r12,
> +			        plus_constant (Pmode, r3, -rounded_size)));
> +
> +
> +      /* Step 3: the loop
> +
> +	while (TEST_ADDR != LAST_ADDR)
> +	  {
> +	    TEST_ADDR = TEST_ADDR + PROBE_INTERVAL
> +	    probe at TEST_ADDR
> +	  }
> +
> +	probes at FIRST + N * PROBE_INTERVAL for values of N from 1
> +	until it is equal to ROUNDED_SIZE.  */
> +
> +      if (TARGET_64BIT && TARGET_LONG64)
> +	emit_insn (gen_probe_stack_rangedi (r3, r3, r12));
> +      else
> +	emit_insn (gen_probe_stack_rangesi (r3, r3, r12));
> +
> +
> +      /* Step 4: probe at FIRST + SIZE if we cannot assert at compile-time
> +	 that SIZE is equal to ROUNDED_SIZE.  */
> +
> +      if (size != rounded_size)
> +	emit_stack_probe (plus_constant (Pmode, r12, rounded_size - size));

I Might Be Wrong, but it looks like this won't probe at FIRST + SIZE
in the case where SIZE == ROUNDED_SIZE, because the loop exits on that
value without probing it.  Should the last line be unconditional,
or does the loop need to be a do-while instead?  (I suppose the latter,
so that there isn't a hole bigger than PROBE_INTERVAL in the
SIZE != ROUNDED_SIZE case?)

> +/* Probe a range of stack addresses from REG1 to REG2 inclusive.  These are
> +   absolute addresses.  */

And as above, it seems to be exclusive of REG2 as things stand.

> +
> +const char *
> +mips_output_probe_stack_range (rtx reg1, rtx reg2)
> +{
> +  static int labelno = 0;
> +  char loop_lab[32], end_lab[32], tmp[64];
> +  rtx xops[2];
> +
> +  ASM_GENERATE_INTERNAL_LABEL (loop_lab, "LPSRL", labelno);
> +  ASM_GENERATE_INTERNAL_LABEL (end_lab, "LPSRE", labelno++);
> +
> +  ASM_OUTPUT_INTERNAL_LABEL (asm_out_file, loop_lab);
> +
> +  /* Jump to END_LAB if TEST_ADDR == LAST_ADDR.  */
> +  xops[0] = reg1;
> +  xops[1] = reg2;
> +  strcpy (tmp, "beq\t%0,%1,");
> +  output_asm_insn (strcat (tmp, &end_lab[1]), xops);
> +
> +  /* TEST_ADDR = TEST_ADDR + PROBE_INTERVAL.  */
> +  xops[1] = GEN_INT (-PROBE_INTERVAL);
> +  if (TARGET_64BIT && TARGET_LONG64)
> +    output_asm_insn (" daddiu\t%0,%0,%1", xops);
> +  else
> +    output_asm_insn (" addiu\t%0,%0,%1", xops);

This only works in noreorder mode.  If there's an asm in the function,
or something else that forces reorder mode (e.g. a -mfix-* option),
the addition won't be put in the delay slot.

"%(%<beq\t%0,%1," and "daddiu\t%0,%0,%1%>%)" should work.  (Note that
our MIPS asm output doesn't have a space before the delay slot; there's
a blank line after it instead.  That's all handled by output_asm_insn though.)

> +  /* Probe at TEST_ADDR and branch.  */
> +  fprintf (asm_out_file, "\tb\t");
> +  assemble_name_raw (asm_out_file, loop_lab);
> +  fputc ('\n', asm_out_file);
> +  if (TARGET_64BIT)
> +    output_asm_insn (" sd\t$0,0(%0)", xops);
> +  else
> +    output_asm_insn (" sw\t$0,0(%0)", xops);

Same here, so I think the branch should use output_asm_insn too.

OK with those changes, thanks.

Richard

Re: [MIPS] Implement static stack checking

[Eric Botcazou]

> This function doesn't work with MIPS16 mode.  Maybe just:
> 
>   if (TARGET_MIPS16)
>     sorry ("MIPS16 stack probes");
> 
> (We can't test TARGET_MIPS16 in something like STACK_CHECK_STATIC_BUILTIN
> because MIPS16ness is a per-function property.)

I put

  if (TARGET_MIPS16)
    sorry ("-fstack-check=specific not implemented for MIPS16");

> Please use MIPS_PROLOGUE_TEMP for r3 (and probably rename r3).
> I suppose GP_REG_FIRST + 12 should be MIPS_PROLOGUE_TEMP2, probably as:
> 
> #define MIPS_PROLOGUE_TEMP2_REGNUM \
>   (TARGET_MIPS16 ? gcc_unreachable () \
>    cfun->machine->interrupt_handler_p ? K1_REG_NUM : GP_REG_FIRST + 12)
> 
> #define MIPS_PROLOGUE_TEMP2(MODE) \
>   gen_rtx_REG (MODE, MIPS_PROLOGUE_TEMP2_REGNUM)
> 
> and update the block comment above the MIPS_PROLOGUE_TEMP_REGNUM definition.

Done.

> I Might Be Wrong, but it looks like this won't probe at FIRST + SIZE
> in the case where SIZE == ROUNDED_SIZE, because the loop exits on that
> value without probing it.  Should the last line be unconditional,
> or does the loop need to be a do-while instead?  (I suppose the latter,
> so that there isn't a hole bigger than PROBE_INTERVAL in the
> SIZE != ROUNDED_SIZE case?)

The loop probes at FIRST + N * PROBE_INTERVAL for values of N from 1 until it 
is equal to ROUNDED_SIZE, inclusive, so FIRST + SIZE is always probed.

> This only works in noreorder mode.  If there's an asm in the function,
> or something else that forces reorder mode (e.g. a -mfix-* option),
> the addition won't be put in the delay slot.
> 
> "%(%<beq\t%0,%1," and "daddiu\t%0,%0,%1%>%)" should work.  (Note that
> our MIPS asm output doesn't have a space before the delay slot; there's
> a blank line after it instead.  That's all handled by output_asm_insn
> though.)

Thanks for the incantation!

> OK with those changes, thanks.

I'll retest with the changes tomorrow.  Thanks for the review.

-- 
Eric Botcazou

Re: [MIPS] Implement static stack checking

[Richard Sandiford]

Eric Botcazou <ebotcazou@adacore.com> writes:
>> I Might Be Wrong, but it looks like this won't probe at FIRST + SIZE
>> in the case where SIZE == ROUNDED_SIZE, because the loop exits on that
>> value without probing it.  Should the last line be unconditional,
>> or does the loop need to be a do-while instead?  (I suppose the latter,
>> so that there isn't a hole bigger than PROBE_INTERVAL in the
>> SIZE != ROUNDED_SIZE case?)
>
> The loop probes at FIRST + N * PROBE_INTERVAL for values of N from 1 until it 
> is equal to ROUNDED_SIZE, inclusive, so FIRST + SIZE is always probed.

Doh!  But in that case, rather than:

1:
	beq	r1,r2,2f
	addiu	r1,r1,interval
	b	1b
	sw	$0,0(r1)
2:

why not just:

1:
	addiu	r1,r1,interval
	bne	r1,r2,1b
	sw	$0,0(r1)

?

Richard

Re: [MIPS] Implement static stack checking

[Eric Botcazou]

> Doh!  But in that case, rather than:
> 
> 1:
> 	beq	r1,r2,2f
> 	addiu	r1,r1,interval
> 	b	1b
> 	sw	$0,0(r1)
> 2:
> 
> why not just:
> 
> 1:
> 	addiu	r1,r1,interval
> 	bne	r1,r2,1b
> 	sw	$0,0(r1)
> 
> ?

The latter will always probe once, the former won't, if ROUNDED_SIZE == 0.

-- 
Eric Botcazou

Re: [MIPS] Implement static stack checking

[Richard Sandiford]

Eric Botcazou <ebotcazou@adacore.com> writes:
>> Doh!  But in that case, rather than:
>> 
>> 1:
>> 	beq	r1,r2,2f
>> 	addiu	r1,r1,interval
>> 	b	1b
>> 	sw	$0,0(r1)
>> 2:
>> 
>> why not just:
>> 
>> 1:
>> 	addiu	r1,r1,interval
>> 	bne	r1,r2,1b
>> 	sw	$0,0(r1)
>> 
>> ?
>
> The latter will always probe once, the former won't, if ROUNDED_SIZE == 0.

But why do we want the loop at all if the rounded size is zero?
It's a compile-time constant after all.

Richard

Re: [MIPS] Implement static stack checking

[Eric Botcazou]

> But why do we want the loop at all if the rounded size is zero?
> It's a compile-time constant after all.

Yes, this never occurs in practice because of the value set for PROBE_INTERVAL 
and STACK_CHECK_PROTECT.  This can only occur in the dynamic case handled in 
explow.c:probe_stack_range.

All the stack checking code, in the middle-end and the various back-ends, use 
a uniform implementation.  This can probably be optimized a bit, but I'm not 
sure it's really worth the hassle.

-- 
Eric Botcazou

Re: [MIPS] Implement static stack checking

[Richard Sandiford]

Eric Botcazou <ebotcazou@adacore.com> writes:
>> But why do we want the loop at all if the rounded size is zero?
>> It's a compile-time constant after all.
>
> Yes, this never occurs in practice because of the value set for PROBE_INTERVAL 
> and STACK_CHECK_PROTECT.  This can only occur in the dynamic case handled in 
> explow.c:probe_stack_range.
>
> All the stack checking code, in the middle-end and the various back-ends, use 
> a uniform implementation.  This can probably be optimized a bit, but I'm not 
> sure it's really worth the hassle.

But wouldn't the target independent "store zero" code (e.g. for alloca)
be emitted at expand time, and optimised in the normal way?  I'd hope
jump threading etc. would then give something like the 3-insn form in
cases where the constant is known to be nonzero.  The problem here is
that we're just writing asm directly, and in that case using the 3-insn
1-branch form seems better than the 4-insn 2-branch one.  It's less
code too :-)

Richard

Re: [MIPS] Implement static stack checking

[Maciej W. Rozycki]

On Mon, 22 Oct 2012, Richard Sandiford wrote:

> > The loop probes at FIRST + N * PROBE_INTERVAL for values of N from 1 until it 
> > is equal to ROUNDED_SIZE, inclusive, so FIRST + SIZE is always probed.
> 
> Doh!  But in that case, rather than:
> 
> 1:
> 	beq	r1,r2,2f
> 	addiu	r1,r1,interval
> 	b	1b
> 	sw	$0,0(r1)
> 2:
> 
> why not just:
> 
> 1:
> 	addiu	r1,r1,interval
> 	bne	r1,r2,1b
> 	sw	$0,0(r1)
> 
> ?

 For the record that can be easily rewritten to support the MIPS16 mode, 
e.g.:

	move	$1,r2
1:
	d/addiu	r1,interval
	li	r2,0
	sw/sd	r2,0(r1)
	move	r2,$1
	cmp	r1,r2
	btnez	1b

with $2 and $3 used as temporaries (in addition to $1) as these are I 
believe available in MIPS16 prologues ($1 obviously is).  The juggling 
with $1 can be avoided if the probe used need not be zero (need it?).  
The range of <interval> supported by the machine instruction encodings 
available is the same as for the standard MIPS or microMIPS mode.

 If we care about MIPS16 support, that is.

  Maciej

Re: [MIPS] Implement static stack checking

[Richard Sandiford]

Sorry, one more thing (obviously a bad night)

Eric Botcazou <ebotcazou@adacore.com> writes:
> +      if (TARGET_64BIT && TARGET_LONG64)
> +	emit_insn (gen_probe_stack_rangedi (r3, r3, r12));
> +      else
> +	emit_insn (gen_probe_stack_rangesi (r3, r3, r12));

Please use:

    emit_insn (PMODE_INSN (gen_probe_stack_range, (r3, r3, r12)));

for this.  The patterns will need to be "_<P:mode>" rather
than just "<P:mode>".

Richard