[PATCH] D64294: [Driver] Consolidate shouldUseFramePointer() and shouldUseLeafFramePointer()

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.


================
Comment at: lib/Driver/ToolChains/Clang.cpp:579
 
-static bool shouldUseFramePointer(const ArgList &Args,
-                                  const llvm::Triple &Triple) {
-  if (Arg *A = Args.getLastArg(options::OPT_fno_omit_frame_pointer,
-                               options::OPT_fomit_frame_pointer))
-    return A->getOption().matches(options::OPT_fno_omit_frame_pointer) ||
-           mustUseNonLeafFramePointerForTarget(Triple);
-
-  if (Args.hasArg(options::OPT_pg))
-    return true;
-
-  return useFramePointerForTargetByDefault(Args, Triple);
-}
-
-static bool shouldUseLeafFramePointer(const ArgList &Args,
-                                      const llvm::Triple &Triple) {
-  if (Arg *A = Args.getLastArg(options::OPT_mno_omit_leaf_frame_pointer,
-                               options::OPT_momit_leaf_frame_pointer))
-    return A->getOption().matches(options::OPT_mno_omit_leaf_frame_pointer);
-
-  if (Args.hasArg(options::OPT_pg))
-    return true;
-
-  if (Triple.isPS4CPU())
-    return false;
-
-  return useFramePointerForTargetByDefault(Args, Triple);
+static FramePointerKind getFramePointerKind(const ArgList &Args,
+                                            const llvm::Triple &Triple) {
----------------
`getFramePointerKind` -> `decideFramePointerKind` / `determineFramePointerKind` ? 


================
Comment at: lib/Driver/ToolChains/Clang.cpp:585
+      (A && A->getOption().matches(options::OPT_fno_omit_frame_pointer)) ||
+      (!(A && A->getOption().matches(options::OPT_fomit_frame_pointer)) &&
+       (Args.hasArg(options::OPT_pg) ||
----------------
It looks better if  `frame_pointer` is represented using tri-state. Something like this?

It would be great to have comments for conditions that are not obvious such as the overriding rules.

```
  // There are three states for frame_pointer.
  enum class FpFlag {true, false, none};
  FpFlag FPF = FpFlag::none;
  if (Arg *A = Args.getLastArg(options::OPT_fomit_frame_pointer,
                               options::OPT_fno_omit_frame_pointer))
    FPF = A->getOption().matches(options::OPT_fno_omit_frame_pointer)) ?
             FpFlag::true : FpFlag::false;

  if (!mustUseNonLeaf && FPF == FpFlag::false)
    return FramePointerKind::None;

  if (mustUseNonLeaf || FPF == FpFlag::true || Args.hasArg(options::OPT_pg) ||
      useFramePointerForTargetByDefault(Args, Triple)) {
    if (Args.hasFlag(options::OPT_momit_leaf_frame_pointer,
                     options::OPT_mno_omit_leaf_frame_pointer,
                     Triple.isPS4CPU()))
      return FramePointerKind::NonLeaf;
    return FramePointerKind::All;
  }
  return FramePointerKind::None;
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D64294