This is an archive of the discontinued LLVM Phabricator instance.

[clang][ARM] PACBTI-M assembly support
ClosedPublic

Authored by stuij on Oct 25 2021, 2:59 AM.

Details

Summary

Introduce assembly support for Armv8.1-M PACBTI extension. This is an optional
extension in v8.1-M.

There are 10 new system registers and 5 new instructions, all predicated on the
feature.

The attribute for llvm-mc is called "pacbti". For armclang, an architecture
extension also called "pacbti" was created.

This patch is part of a series that adds support for the PACBTI-M extension of
the Armv8.1-M architecture, as detailed here:

https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/armv8-1-m-pointer-authentication-and-branch-target-identification-extension

The PACBTI-M specification can be found in the Armv8-M Architecture Reference
Manual:

https://developer.arm.com/documentation/ddi0553/latest

The following people contributed to this patch:

  • Victor Campos
  • Ties Stuij

Diff Detail

Event Timeline

stuij created this revision.Oct 25 2021, 2:59 AM
stuij requested review of this revision.Oct 25 2021, 2:59 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptOct 25 2021, 2:59 AM
ostannard added inline comments.
llvm/lib/Target/ARM/ARMInstrThumb2.td
4079

Why are these needed in addition to the PACBTIHintSpaceInst instructions below?

5696

I think all of the AUT instructions need hasSideEffects set, since they can raise exceptions.

5697

This needs isBranch set, and a test added to test/MC/ARM/implicit-it-generation.s.

llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
6441

PACG, AUTG and BXAUT can be conditional, so shouldn't be in this list.

6589

Again, some of these are predicable.

12281

Unintentional formatting change.

llvm/test/MC/ARM/armv8.1m-pacbti-error.s
4

We should also test the cases where PACG/AUTG/BXAUT cannot use PC/SP.

vhscampos added inline comments.Oct 28 2021, 7:29 AM
llvm/lib/Target/ARM/ARMInstrThumb2.td
4079

Since these instructions are in the HINT space, without specifying inst aliases, they were printed in the HINT form when using llvm-mc instead of in their PACBTI form. Therefore I added these t2InstAlias instances.

stuij updated this revision to Diff 384721.Nov 4 2021, 5:39 AM

addressed review comments:

  • Added predicate operand to bxaut, autg. They will now also behave correctly in IT blocks.
  • Adjusted operator constraints for a number of instructions so these confirm to the specs.
  • Expanded tests to test above changes and permutation of allowed operators for instructions.
  • Addressed minor review comments.
stuij marked 6 inline comments as done.Nov 4 2021, 5:49 AM

A slight amendment to the description of the just uploaded patch amendment: PACG has also been made conditional.

llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
6441

Thanks. Besides removing these from this list and the one below, I've added a predicate operator to actually make them conditional. The description of the patch amend didn't mention PACG, but it too has been made conditional.

llvm/test/MC/ARM/armv8.1m-pacbti-error.s
4

Thanks. Adding extra testcases uncovered a number of cases where the changes didn't match the spec. This should now be resolved.

labrinea accepted this revision.Nov 26 2021, 7:59 AM
labrinea added a subscriber: labrinea.

Looks like you've addressed Oliver's comments. I don't have any new suggestions from my end. Just make sure you've removed the test xfail before merging.

clang/test/Driver/darwin-ld-lto.c
2 ↗(On Diff #384721)

Seems accidental.

This revision is now accepted and ready to land.Nov 26 2021, 7:59 AM
stuij updated this revision to Diff 390314.Nov 29 2021, 5:38 AM
stuij marked 2 inline comments as done.

addressed review comment and clang-format suggestions

This revision was landed with ongoing or failed builds.Nov 30 2021, 1:29 AM
This revision was automatically updated to reflect the committed changes.