[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