On Fri, Oct 11, 2019 at 04:44:51PM -0500, Segher Boessenkool wrote:
> On Wed, Oct 09, 2019 at 04:22:06PM -0400, Michael Meissner wrote:
> > This patch allows -fstack-protect to work with large stack frames.
>
> I don't understand; please explain.
>
> What I see is a workaround for not properly handling prefixed addresses
> in the stack protect code (by forcing the addresses to not be prefixed at
> expand time).
Several iterations ago, you explicitly told me to not to allow prefixed insns
here. So I made it to use traditional instructions.
If you recall the code was:
(define_insn "stack_protect_testdi"
[(set (match_operand:CCEQ 0 "cc_reg_operand" "=x,?y")
(unspec:CCEQ [(match_operand:DI 1 "memory_operand" "Y,Y")
(match_operand:DI 2 "memory_operand" "Y,Y")]
UNSPEC_SP_TEST))
(set (match_scratch:DI 4 "=r,r") (const_int 0))
(clobber (match_scratch:DI 3 "=&r,&r"))]
"TARGET_64BIT"
{
if (prefixed_mem_operand (operands[1], DImode))
output_asm_insn ("pld %3,%1", operands);
else
output_asm_insn ("ld%U1%X1 %3,%1", operands);
if (prefixed_mem_operand (operands[2], DImode))
output_asm_insn ("pld %4,%2", operands);
else
output_asm_insn ("ld%U2%X2 %4,%2", operands);
if (which_alternative == 0)
output_asm_insn ("xor. %3,%3,%4", operands);
else
output_asm_insn ("cmpld %0,%3,%4\;li %3,0", operands);
return "li %4,0";
}
;; Back to back prefixed memory instructions take 20 bytes (8 bytes for each
;; prefixed instruction + 4 bytes for the possible NOP).
[(set (attr "length")
(cond [(and (match_operand 1 "prefixed_mem_operand")
(match_operand 2 "prefixed_mem_operand"))
(if_then_else (eq_attr "alternative" "0")
(const_string "28")
(const_string "32"))
(ior (match_operand 1 "prefixed_mem_operand")
(match_operand 2 "prefixed_mem_operand"))
(if_then_else (eq_attr "alternative" "0")
(const_string "20")
(const_string "24"))]
(if_then_else (eq_attr "alternative" "0")
(const_string "16")
(const_string "20"))))
(set_attr "prefixed" "no")])
It can't use the normal prefixed support used in other insns because this one
has multiple insns (hence the 'p' trick for ASM_OUTPUT_OPCODE won't work) and
two memory addresses.
> > +rtx
> > +make_memory_non_prefixed (rtx mem)
> > +{
> > + gcc_assert (MEM_P (mem));
> > +
> > + rtx old_addr = XEXP (mem, 0);
> > + if (address_is_prefixed (old_addr, GET_MODE (mem), NON_PREFIXED_DEFAULT))
> > + {
> > + rtx new_addr;
> > +
> > + if (GET_CODE (old_addr) == PLUS
> > + && (REG_P (XEXP (old_addr, 0)) || SUBREG_P (XEXP (old_addr, 0)))
>
> How can you ever have a subreg in an address? One in Pmode even?
I could imagine having a union or something similar that generates a subreg.
>
> > + && CONST_INT_P (XEXP (old_addr, 1)))
> > + {
> > + rtx tmp_reg = force_reg (Pmode, XEXP (old_addr, 1));
> > + new_addr = gen_rtx_PLUS (Pmode, XEXP (old_addr, 0), tmp_reg);
> > + }
> > + else
> > + new_addr = force_reg (Pmode, old_addr);
> > +
> > + mem = change_address (mem, VOIDmode, new_addr);
>
> replace_equiv_address ?
>
> > +(define_expand "stack_protect_setdi"
> > + [(parallel [(set (match_operand:DI 0 "memory_operand")
> > + (unspec:DI [(match_operand:DI 1 "memory_operand")]
> > + UNSPEC_SP_SET))
> > + (set (match_scratch:DI 2)
> > + (const_int 0))])]
> > + "TARGET_64BIT"
> > +{
> > + if (TARGET_PREFIXED_ADDR)
> > + {
> > + operands[0] = make_memory_non_prefixed (operands[0]);
> > + operands[1] = make_memory_non_prefixed (operands[1]);
> > + }
> > +})
>
> It shouldn't be terribly hard to make the define_insns just *work* with
> prefixed insns, instead? Is there any reason we are sure these memory
> references will not turn into something that needs prefixed insns, after
> expand? It seems fragile like this.
As I said, I've done it in the past. It is complicated because this insn must
generate two or more insns without spliting. But it is doable.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: [email protected], phone: +1 (978) 899-4797
