On 07/23/2018 08:33 AM, Richard Earnshaw (lists) wrote: > [sorry, missed this mail somehow] > > On 11/07/18 22:01, Jeff Law wrote: >> On 07/09/2018 10:38 AM, Richard Earnshaw wrote: >>> This patch is the main part of the speculation tracking code. It adds >>> a new target-specific pass that is run just before the final branch >>> reorg pass (so that it can clean up any new edge insertions we make). >>> The pass is only run with -mtrack-speculation is passed on the command >>> line. >>> >>> One thing that did come to light as part of this was that the stack pointer >>> register was not being permitted in comparision instructions. We rely on >>> that for moving the tracking state between SP and the scratch register at >>> function call boundaries. >> Note that the sp in comparison instructions issue came up with the >> improvements to stack-clash that Tamar, Richard S. and you worked on. >> > > I can certainly lift that part into a separate patch. Your call. It was mostly an observation that the change was clearly needed elsewhere. I'm certainly comfortable letting that hunk go in with whichever kit is approved first :-)
> >> >>> >>> * config/aarch64/aarch64-speculation.cc: New file. >>> * config/aarch64/aarch64-passes.def (pass_track_speculation): Add before >>> pass_reorder_blocks. >>> * config/aarch64/aarch64-protos.h (make_pass_track_speculation): Add >>> prototype. >>> * config/aarch64/aarch64.c (aarch64_conditional_register_usage): Fix >>> X14 and X15 when tracking speculation. >>> * config/aarch64/aarch64.md (register name constants): Add >>> SPECULATION_TRACKER_REGNUM and SPECULATION_SCRATCH_REGNUM. >>> (unspec): Add UNSPEC_SPECULATION_TRACKER. >>> (speculation_barrier): New insn attribute. >>> (cmp<mode>): Allow SP in comparisons. >>> (speculation_tracker): New insn. >>> (speculation_barrier): Add speculation_barrier attribute. >>> * config/aarch64/t-aarch64: Add make rule for aarch64-speculation.o. >>> * config.gcc (aarch64*-*-*): Add aarch64-speculation.o to extra_objs. >>> * doc/invoke.texi (AArch64 Options): Document -mtrack-speculation. >>> --- >>> gcc/config.gcc | 2 +- >>> gcc/config/aarch64/aarch64-passes.def | 1 + >>> gcc/config/aarch64/aarch64-protos.h | 3 +- >>> gcc/config/aarch64/aarch64-speculation.cc | 494 >>> ++++++++++++++++++++++++++++++ >>> gcc/config/aarch64/aarch64.c | 13 + >>> gcc/config/aarch64/aarch64.md | 30 +- >>> gcc/config/aarch64/t-aarch64 | 10 + >>> gcc/doc/invoke.texi | 10 +- >>> 8 files changed, 558 insertions(+), 5 deletions(-) >>> create mode 100644 gcc/config/aarch64/aarch64-speculation.cc >> Given the consensus forming about using these kind of masking >> instructions being the preferred way to mitigate (as opposed to lfence >> barriers and the like) I have to ask your opinions about making the bulk >> of this a general pass rather than one specific to the aarch backend. >> I'd hate to end up duplicating all this stuff across multiple architectures. >> >> I think it all looks pretty reasonable though. >> >> jeff >> > > > It would be nice to make this more generic, but I'm not sure how easy > that would be. Some of the analysis is surely the same, but deployment > of the mitigation itself is perhaps more complex. At this point in > time, I think I'd prefer to go with the target-specific implementation > and then look to generalize it as a follow-up. There may be some more > optimizations to add later as well. ACK. I suspect it's mostly the analysis side that we'll want to share. I don't mind giving you the advantage of going first and letting it live in the aarch64 backend. Second implementation can extract the analysis bits :-) So IMHO, this can go forward whenever you want to push it. Jeff
