[PATCH] D90868: [IR] Define @llvm.ptrauth intrinsics.

[Pablo Barrio via Phabricator via llvm-commits]

pbarrio added inline comments.


================
Comment at: llvm/docs/PointerAuth.md:17-19
+It is implemented by the [AArch64 target](#aarch64-support), using the
+[ARMv8.3 Pointer Authentication Code](#armv8-3-pointer-authentication-code)
+instructions, to support the Darwin arm64e ABI.
----------------
Since at least part of the implementation is independent of the architecture, would it make sense to reword to something like: "The initial/current/original implementation leverages the [ARMv8.3 Pointer Authentication Code](#armv8-3-pointer-authentication-code)
instructions on the [AArch64 back-end](#aarch64-support), and follows the rules defined in the Darwin arm64e ABI". As it stands now, it gave me the impression that it is bespoke work for AArch64 and arm64e, when in fact it is actually quite independent and extensible to other architectures and ABIs (which is great BTW).

Also ok to leave it as-is, and only change it once support for other ABIs (or architectures) appears.


================
Comment at: llvm/docs/PointerAuth.md:22
+
+## Concepts
+
----------------
Maybe I'm jumping a bit ahead of myself here.

I was reading the fully-patched toolchain (e.g. here: https://github.com/pcc/llvm-project/tree/apple-pac3) and I noticed most of these concepts are explained in clang/docs/PointerAuthentication.rst in more detail. Should we have them only in one place and reference the other document? I really like how the Clang document explains the high-level workings of PAuth, and this document seems more focused on the internal implementation in LLVM. Ofc you may have other plans for the two-document split :)

For example, we could just point out the correspondence with the other document in the next section:


```
## LLVM IR Representation

### Intrinsics

The intrinsics implement the three fundamental operations in PAuth (sign, auth, and strip), as well as a bundle (resign), a generic data signing and an operation to help generate different types of discriminators (blend). For more information on PAuth operations, check out <link to Clang document Section "Basic concepts">.  Also, <link to Clang document Section "Discriminators"> explains discriminators in detail.
```
Or something like that.

I would expect anyone interested in this document to have read the other document first, or at least similar Arm documents, so they should already be familiar with how PAuth works.


================
Comment at: llvm/docs/PointerAuth.md:288
+AArch64 is currently the only target with full support of the pointer
+authentication primitives, based on ARMv8.3 instructions.
+
----------------
Nit: replace ARMv8.3 by Armv8.3-A to follow Arm's trademark guidelines: https://www.arm.com/company/policies/trademarks/arm-trademark-list/arm-trademark. There are a few more occurrences of this in the patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90868/new/

https://reviews.llvm.org/D90868