[PATCH] D143285: X86: completely refactor `X86FrameLowering::emitPrologue`
Sergei Barannikov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat May 13 03:44:52 PDT 2023
barannikov88 added a comment.
FrameBuilder class should be stateless. I.e. all member functions should be `const` (and there should be no `mutable` variables).
================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:1512
+
+ void EmitPrologue() {
+ int SEHFrameOffset = 0;
----------------
Method names should start with a lowercase letter.
================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:1549
+
+ ~FrameBuilder() {
+ // At this point we know if the function has WinCFI or not.
----------------
Destructors should not have side effects.
================
Comment at: llvm/lib/Target/X86/X86FrameLowering.cpp:1567
// combined with the frame pointer directly.
- BuildMI(MBB, MBBI, DL, TII.get(X86::OR64rm), MachineFramePtr)
- .addUse(MachineFramePtr)
+ BuildMI(MBB, MBBI, DL, TII.get(X86::OR64rm), MachineFramePointer)
+ .addUse(MachineFramePointer)
----------------
This renaming does not looks necessary, avoiding it would reduce the patch size.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143285/new/
https://reviews.llvm.org/D143285
More information about the llvm-commits
mailing list