[PATCH] D56353: Replace cc1 options '-mdisable-fp-elim' and '-momit-leaf-frame-pointer' with '-mframe-pointer'

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.


================
Comment at: clang/include/clang/Basic/CodeGenOptions.h:120
 
+  enum FramePointerKind {
+    FP_All,         // Keep all frame pointers.
----------------
We can probably use `enum class FramePointerKind { None, NonLeaf, All };` here.. (`enum class ClassABI` is an example in this file)


================
Comment at: clang/lib/CodeGen/CGCall.cpp:1723
+    else
+      llvm_unreachable("unknown frame-pointer");
 
----------------
`llvm_unreachable` can be deleted if a switch statement is used.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:49
 
+using FramePointerKind = clang::CodeGenOptions::FramePointerKind;
 using namespace clang::driver;
----------------
This is probably not necessary. The enum is only used in 2 places below.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3961
+  else
+    llvm_unreachable("unknown FramePointerKind");
+
----------------
`llvm_unreachable("unknown FramePointerKind");` can be deleted if a switch statement is used.


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:845
+  if (Args.hasArg(OPT_pg))
+    Opts.setFramePointer(CodeGenOptions::FP_All);
+
----------------
I guess this can be deleted now.

`-pg` + FramePointerKind::None is rejected by the driver.

```
  if (Arg *A = Args.getLastArg(options::OPT_pg))
    if (FPKeepKind == FramePointerKind::None)
      D.Diag(diag::err_drv_argument_not_allowed_with) << "-fomit-frame-pointer"
                                                      << A->getAsString(Args);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56353