[PATCH] D159357: [AArch64] Move PAuth codegen down the machine pipeline

[Anatoly Trosinenko via Phabricator via llvm-commits]

atrosinenko added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64PointerAuth.cpp:33-42
+  MachineFunction *MF;
+
+  const AArch64Subtarget *Subtarget;
+  const AArch64FunctionInfo *MFnI;
+  const AArch64InstrInfo *TII;
+
+  bool UseBKey;
----------------
pratlucas wrote:
> I feel those member variables could be removed to make the class less stateful.
> A single instantiation of this pass will be used over multiple machine functions and most of those members are specific to each of the functions. Removing them would also allow the private methods below to be made freestanding static functions instead.
Not sure if it is necessary to expose `signLR` and `authenticateLR` as public static methods right now, so kept `Subtarget` and `TII` fields (this seems to be done by some other passes as well). Dropped other "cached" fields. Though, I don't mind making `signLR` and `authenticateLR` methods static if needed.


================
Comment at: llvm/lib/Target/AArch64/AArch64PointerAuth.cpp:157
+      auto It = MI.getIterator();
+      bool HasWinCFI = false;
+      switch (MI.getOpcode()) {
----------------
tmatheson wrote:
> Looks like HasWinCFI is not used for anything but the asserts below, it could be removed. It has always been a confusing part of this code.
Agree, dropped the assertions.

Here are some relevant patches that I found when investigating how WinCFI is handled:
* D61960 (correctness) computes the actual presence of WinCFI directives
* D88641 (binary size) optimizes `emitEpilogue` to not emit WinCFI directives if prologue doesn't have any.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159357