[PATCH] D159357: [AArch64] Move PAuth codegen down the machine pipeline
Anatoly Trosinenko via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 19 08:45:26 PDT 2023
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
More information about the llvm-commits
mailing list