[PATCH] D132385: [AArch64][PAC] Select XPAC for ptrauth.strip intrinsic.

[Kristof Beyls via Phabricator via llvm-commits]

kristof.beyls accepted this revision.
kristof.beyls added a comment.
This revision is now accepted and ready to land.

LGTM!
I just added a few minor nitpicks - I'll leave it to you to decide if following those suggestions would result in an improvement or not.



================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.h:514
+
+/// Return AUT opcode to be used for a ptrauth auth using the given key, or its
+/// AUT*Z variant that doesn't take a discriminator operand, using zero instead.
----------------
nitpick, maybe do s/ptrauth auth/pointer authentication/?


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.h:526
+
+/// Return PAC opcode to be used for a ptrauth auth using the given key, or its
+/// PAC*Z variant that doesn't take a discriminator operand, using zero instead.
----------------
nitpick, maybe do s/ptrauth auth/pointer signing/?


================
Comment at: llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h:798
+  DA = 2,
+  DB = 3
+};
----------------
I guess that if we added "LAST = 3" here, the somewhat cryptic if statement in AArch64InstructionSelector.cpp that says

```
if (Key > 3)
      return false;
```
could instead say something like

```
if (AArch64PACKey::LAST > 3)
      return false;
```

It seems the word "LAST" is already used in a few other places in LLVM to indicate the highest defined value in an enum. Maybe doing this change would be a minor improvement in readability/maintainability?


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

https://reviews.llvm.org/D132385