Re: [PATCH 10/15 V4] arm: Implement cortex-M return signing address codegen

[Andrea Corallo <andrea.corallo@arm.com>]

Hi Richard,

thanks for reviewing.

Just one clarification before I complete the respin of this patch.

Richard Earnshaw <Richard.Earnshaw@foss.arm.com> writes:

[...]

> Also, I think (out of an abundance of caution) we really need a
> scheduling barrier placed before calls to gen_aut_nop() pattern is
> emitted, to ensure that the scheduler never tries to move this
> instruction away from the position we place it.  Use gen_blockage()
> for that (see TARGET_SCHED_PROLOG).  Alternatively, we could make the
> UNSPEC_PAC_NOP an unspec_volatile, which has the same effect (IIRC)
> without needing an additional insn - if you use this approach, then
> please make sure this is explained in a comment.
>
> +(define_insn "pacbti_nop"
> +  [(set (reg:SI IP_REGNUM)
> +	(unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)]
> +		   UNSPEC_PACBTI_NOP))]
> +  "arm_arch8m_main"
> +  "pacbti\t%|ip, %|lr, %|sp"
> +  [(set_attr "conds" "unconditional")])
>
> The additional side-effect of this being a BTI landing pad means that
> we mustn't move any other instruction before it.  So I think this
> needs to be an unspec_volatile as well.

IIUC from this we want to make all the three (UNSPEC_PAC_NOP,
UNSPEC_PACBTI_NOP, UNSPEC_AUT_NOP) unspec volatile, correct?

IIUC correctly the scheduler should not reorder them as we have
expressed which register they consume and produce but is for double
caution correct?

> On the tests, they are OK as they stand, but we lack anything that
> will be tested when suitable hardware is unavailable (all tests are
> "dg-do run").  Can we please have some compile-only tests as well?

Ack.

BR

  Andrea