On Fri, Nov 01, 2019 at 10:22:03PM -0500, Segher Boessenkool wrote: > Hi! > > On Wed, Oct 16, 2019 at 09:47:41AM -0400, Michael Meissner wrote: > > This patch fixes the stack protection insns to support stacks larger than > > 16-bits on the 'future' system using prefixed loads and stores. > > > +;; We can't use the prefixed attribute here because there are two memory > > +;; instructions. We can't split the insn due to the fact that this > > operation > > +;; needs to be done in one piece. > > (define_insn "stack_protect_setdi" > > [(set (match_operand:DI 0 "memory_operand" "=Y") > > (unspec:DI [(match_operand:DI 1 "memory_operand" "Y")] UNSPEC_SP_SET)) > > (set (match_scratch:DI 2 "=&r") (const_int 0))] > > "TARGET_64BIT" > > - "ld%U1%X1 %2,%1\;std%U0%X0 %2,%0\;li %2,0" > > +{ > > + if (prefixed_memory (operands[1], DImode)) > > + output_asm_insn ("pld %2,%1", operands); > > + else > > + output_asm_insn ("ld%U1%X1 %2,%1", operands); > > + > > + if (prefixed_memory (operands[0], DImode)) > > + output_asm_insn ("pstd %2,%0", operands); > > + else > > + output_asm_insn ("std%U0%X0 %2,%0", operands); > > We could make %pN mean 'p' for prefixed, for memory as operands[N]? Are > there more places than this that could use that? How about inline asm?
At the moment, I did not add this. We can revisit it later. > > + (set (attr "length") > > + (cond [(and (match_operand 0 "prefixed_memory") > > + (match_operand 1 "prefixed_memory")) > > + (const_string "24") > > + > > + (ior (match_operand 0 "prefixed_memory") > > + (match_operand 1 "prefixed_memory")) > > + (const_string "20")] > > + > > + (const_string "12")))]) > > You can use const_int instead of const_string here, I think? Please do > that if it works. > > Quite a simple expression, phew :-) Const_int works. > > + if (which_alternative == 0) > > + output_asm_insn ("xor. %3,%3,%4", operands); > > + else > > + output_asm_insn ("cmpld %0,%3,%4\;li %3,0", operands); > > That doesn't work: the backslash is treated like the escape character, in > a C block. I think doubling it will work? Check the generated insn-output.c, > it should be translated to \t\n in there. > > Okay for trunk with those things taken care of. Thanks! As we discussed, this does work. Here is the patch committed. I did a bootstrap and did make check. There were no regressions. 2019-11-11 Michael Meissner <meiss...@linux.ibm.com> * config/rs6000/predicates.md (prefixed_memory): New predicate. * config/rs6000/rs6000.md (stack_protect_setdi): Deal with either address being a prefixed load/store. (stack_protect_testdi): Deal with either address being a prefixed load. Index: gcc/config/rs6000/predicates.md =================================================================== --- gcc/config/rs6000/predicates.md (revision 278062) +++ gcc/config/rs6000/predicates.md (working copy) @@ -1828,3 +1828,10 @@ (define_predicate "pcrel_external_addres (define_predicate "pcrel_local_or_external_address" (ior (match_operand 0 "pcrel_local_address") (match_operand 0 "pcrel_external_address"))) + +;; Return true if the operand is a memory address that uses a prefixed address. +(define_predicate "prefixed_memory" + (match_code "mem") +{ + return address_is_prefixed (XEXP (op, 0), mode, NON_PREFIXED_DEFAULT); +}) Index: gcc/config/rs6000/rs6000.md =================================================================== --- gcc/config/rs6000/rs6000.md (revision 278062) +++ gcc/config/rs6000/rs6000.md (working copy) @@ -11536,14 +11536,44 @@ (define_insn "stack_protect_setsi" [(set_attr "type" "three") (set_attr "length" "12")]) +;; We can't use the prefixed attribute here because there are two memory +;; instructions. We can't split the insn due to the fact that this operation +;; needs to be done in one piece. (define_insn "stack_protect_setdi" [(set (match_operand:DI 0 "memory_operand" "=Y") (unspec:DI [(match_operand:DI 1 "memory_operand" "Y")] UNSPEC_SP_SET)) (set (match_scratch:DI 2 "=&r") (const_int 0))] "TARGET_64BIT" - "ld%U1%X1 %2,%1\;std%U0%X0 %2,%0\;li %2,0" +{ + if (prefixed_memory (operands[1], DImode)) + output_asm_insn ("pld %2,%1", operands); + else + output_asm_insn ("ld%U1%X1 %2,%1", operands); + + if (prefixed_memory (operands[0], DImode)) + output_asm_insn ("pstd %2,%0", operands); + else + output_asm_insn ("std%U0%X0 %2,%0", operands); + + return "li %2,0"; +} [(set_attr "type" "three") - (set_attr "length" "12")]) + + ;; Back to back prefixed memory instructions take 20 bytes (8 bytes for each + ;; prefixed instruction + 4 bytes for the possible NOP). Add in 4 bytes for + ;; the LI 0 at the end. + (set_attr "prefixed" "no") + (set_attr "num_insns" "3") + (set (attr "length") + (cond [(and (match_operand 0 "prefixed_memory") + (match_operand 1 "prefixed_memory")) + (const_int 24) + + (ior (match_operand 0 "prefixed_memory") + (match_operand 1 "prefixed_memory")) + (const_int 20)] + + (const_int 12)))]) (define_expand "stack_protect_test" [(match_operand 0 "memory_operand") @@ -11582,6 +11612,9 @@ (define_insn "stack_protect_testsi" lwz%U1%X1 %3,%1\;lwz%U2%X2 %4,%2\;cmplw %0,%3,%4\;li %3,0\;li %4,0" [(set_attr "length" "16,20")]) +;; We can't use the prefixed attribute here because there are two memory +;; instructions. We can't split the insn due to the fact that this operation +;; needs to be done in one piece. (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") @@ -11590,10 +11623,45 @@ (define_insn "stack_protect_testdi" (set (match_scratch:DI 4 "=r,r") (const_int 0)) (clobber (match_scratch:DI 3 "=&r,&r"))] "TARGET_64BIT" - "@ - ld%U1%X1 %3,%1\;ld%U2%X2 %4,%2\;xor. %3,%3,%4\;li %4,0 - ld%U1%X1 %3,%1\;ld%U2%X2 %4,%2\;cmpld %0,%3,%4\;li %3,0\;li %4,0" - [(set_attr "length" "16,20")]) +{ + if (prefixed_memory (operands[1], DImode)) + output_asm_insn ("pld %3,%1", operands); + else + output_asm_insn ("ld%U1%X1 %3,%1", operands); + + if (prefixed_memory (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). Add in either 4 or + ;; 8 bytes to do the test. + [(set_attr "prefixed" "no") + (set_attr "num_insns" "4,5") + (set (attr "length") + (cond [(and (match_operand 1 "prefixed_memory") + (match_operand 2 "prefixed_memory")) + (if_then_else (eq_attr "alternative" "0") + (const_int 28) + (const_int 32)) + + (ior (match_operand 1 "prefixed_memory") + (match_operand 2 "prefixed_memory")) + (if_then_else (eq_attr "alternative" "0") + (const_int 20) + (const_int 24))] + + (if_then_else (eq_attr "alternative" "0") + (const_int 16) + (const_int 20))))]) ;; Here are the actual compare insns. -- 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