Re: [PATCH] V6, #4 of 17: Add prefixed instruction support to stack protect insns

[Michael Meissner <meissner at linux dot ibm dot com>]

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?

Right now, the only two places that do this are the two stack protect insns.
Everything else that I'm aware of that generates multiple loads or stores will
do a split before final.

> > +   (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.

I'll try it out on Monday.

> Quite a simple expression, phew :-)
> 
> > +  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.

Yes it does work.  I just checked.

> Okay for trunk with those things taken care of.  Thanks!

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.ibm.com, phone: +1 (978) 899-4797