Hi Richard, thanks for reviewing.
Just one clarification before I complete the respin of this patch. Richard Earnshaw <richard.earns...@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
