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: meiss...@linux.ibm.com, phone: +1 (978) 899-4797