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

[Ahmed Bougacha via Phabricator via llvm-commits]

ab added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp:1227
+       // XPACI $x0.
+       MachineInstrBuilder DefMIB =
+           BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(AArch64::ORRXrs))
----------------
apazos wrote:
> there is no unit test case for this scenario
Yeah, `XPACIuntied` is really intended to be used in `@llvm.returnaddress` when return address signing (/PAC-RET) is enabled.  I pulled it out of this patch into that one, where it actually gets exercised.


================
Comment at: llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp:5859
+    if (Key > 3)
+      return false;
+    unsigned Opcode =
----------------
kristof.beyls wrote:
> tschuett wrote:
> > Can the Key be 2? Is an if-statement easier to comprehend than a ternary operator?
> I guess having an enum for the valid key descriptors for AArch64 would make this and other code working with keys more readable. I'm not sure if that's typically done or where to define that enum?
> Than in this location, maybe a switch on that enum would be clearer?
> 
> I see these constants 0,1,2,3 are also needed in tablegen files. Not sure if something similar to an enum exists in Tablegen?
A later patch in the series actually does introduce helper functions in AArch64BaseInfo.h/AArch64InstrInfo.h, so it's a simple matter of using them here!  I combined the two in this review, to be committed separately.


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

https://reviews.llvm.org/D132385