Eric Botcazou <ebotca...@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
