Raise -fomit-frame-pointer into the Clang driver?

[DanAlbert]

The only identified size difference between GCC and Clang remaining is that GCC defaulted to -fomit-frame-pointer whereas Clang does not: #133

Should we enable this by default in the driver for Android targets? iirc ASan needs frame pointers to give reasonable performance, so perhaps not.

@rprichard @stephenhines @pirama-arumuga-nainar

[DanAlbert]

Clang's defaults here are weird. For Linux targets (including Android), frame pointers are disabled when optimizations are enabled on x86/x86_64 (and a bunch of other things we don't support), but always on for arm/arm64.

My instinct is to not differ from existing Linux behavior, but given that this is free code size (and the few users that do need it can enable it, we can re-enable if asan is used, etc), maybe we should?

Apple targets don't support the flag at all. I couldn't work out whether they were always on or always off.

Triaging to r23 to have either send the patch upstream or close won't fix by then.

[enh-google]

https://developer.apple.com/library/archive/documentation/Xcode/Conceptual/iPhoneOSABIReference/Articles/ARM64FunctionCallingConventions.html says:

"The frame pointer register (x29) must always address a valid frame record, although some functions—such as leaf functions or tail calls—may elect not to create an entry in this list. As a result, stack traces will always be meaningful, even without debug information."

which is what i'd want as a developer too. i support having people explicitly opt-out of frame pointers.

[DanAlbert]

i support having people explicitly opt-out of frame pointers.

In other words we should upstream a patch, but one to change x86/x86_64 to be always on?

BTW, the leaf node behavior is default for clang as far as I can tell: https://github.com/llvm/llvm-project/blob/9d2d6e71f096ad43b178c576adf94fc922034c73/clang/lib/Driver/ToolChains/Clang.cpp#L594

[DanAlbert]

Actually, I think the leaf node behavior is only for some targets. Here are some more relevant sections: https://github.com/llvm/llvm-project/blob/9d2d6e71f096ad43b178c576adf94fc922034c73/clang/lib/Driver/ToolChains/Clang.cpp#L527 https://github.com/llvm/llvm-project/blob/9d2d6e71f096ad43b178c576adf94fc922034c73/clang/lib/Driver/ToolChains/Clang.cpp#L515

[enh-google]

In other words we should upstream a patch, but one to change x86/x86_64 to be always on?

oh, interesting. i hadn't thought that far ahead. i'm leaning towards "leave x86-64 alone until we have a reason to change it"?

[nickdesaulniers]

I've touched this part of Clang multiple times. What specifically is the issue here? Note that we do not implicitly set -fomit-frame-pointer for 32b ARM android targets. useFramePointerForTargetByDefault is the relevant function in Clang.

[DanAlbert]

Just hadn't come to a conclusion on whether we wanted to change this, and if we did how we'd want to do that. Whether we want to be consistent with Apple, with GNU, or with ourselves across architectures was undecided. Mostly no decision because no one had strong feelings :)

[stephenhines]

http://b/157669376 has a larger discussion about whether we should change things (re: frame pointers) for Android platform builds. I do not think we should be adjusting the Android defaults here.

[nickdesaulniers]

I think it's ok to close this bug then. Please feel free to reopen it though!

[HaiPhan2002]

Hello @DanAlbert,
As the discussion here, fno-omit-frame-pointer is default for Android? for all android-ndk versions until now?
Thanks,
Hai

[DanAlbert]

Idk about all versions but I believe that's currently the case. I'm not even sure about non-arm32 targets.

[HaiPhan2002]

Thank you for information.