[PATCH] D74698: [CodeGen] -pg shouldn't add "frame-pointer"="all" fn attr w/ -mfentry

Tue Feb 18 11:21:49 PST 2020

nickdesaulniers added a comment.

In D74698#1881034 <https://reviews.llvm.org/D74698#1881034>, @MaskRay wrote:

> When -mfentry is specified, why should frame pointers be disabled?

It doesn't disable them; `-pg` was force enabling them for all functions, when in GCC does not enable them for the combination of `-pg -mfentry`.  This patch is simply matching the behavior of GCC.  If you want the existing behavior, then `-pg -mfentry -fno-omit-frame-pointer` works with this patch applied.  `-pg` should not be setting `-fno-omit-frame-pointers` in the presence of `-mfentry`.

> Is that because the Linux kernel has assumption about the exact code sequence? `call __fentry__` is the first instruction. Isn't that sufficient?

It's not that the current implementation is broken or doesn't work, it's that it's inefficient from the Kernel's perspective.  The kernel does not want frame pointers as it has its own means for unwinding (though there are many unwinders in the kernel, sometimes per architecture, some do rely on frame pointers but explicitly add `-fno-omit-frame-pointer` if necessary). When configuring a kernel to be able to trace kernel execution at runtime, `-pg -mfentry` are added for x86_64, specifically because they don't add frame pointers.  So it seems like Clang is strictly worse than GCC in this regard, as when enabling kernel tracing, suddenly clang starts emitting unwanted frame pointer instructions.

> (This may be another example demonstrating that piggybacking an option (-mfentry) on top of an existing one (-pg) can turn out to be a bad idea...)

Agreed.

Also, more context:
https://www.linuxjournal.com/content/simplifying-function-tracing-modern-gcc
https://lore.kernel.org/patchwork/patch/1072232/
https://linux.kernel.narkive.com/X1y4Jcj4/rfc-how-to-handle-function-tracing-frame-pointers-and-mfentry

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74698