1517816 - Enable -fwhole-program-vtables for clang builds

Status: NEW

(Firefox Build System :: General, enhancement, P3)

Description

[June Wilde (she/her) [:jewilde]]

Clang has a command line flag (-fwhole-program-vtables) for performing whole-program devirtualization on vtables in builds utilizing link time optimization

This would net us some small security and performance gains

Comment 1

[Chris Peterson [:cpeterson]]

clang's -fforce-emit-vtables [1] (GCC already does this by default) and -fstrict-vtable-pointers [2] flags may also allow the compiler to devirtualize more virtual function calls.

[1] http://releases.llvm.org/7.0.0/tools/clang/docs/ReleaseNotes.html#new-compiler-flags and https://reviews.llvm.org/D47108
[2] http://blog.llvm.org/2017/03/devirtualization-in-llvm-and-clang.html

Comment 2

[Jan Hubicka]

I am not familiar with -fwhole-program-vtables in detail. But documentation says "These features use whole-program information, so they require the entire class hierarchy to be visible in order to work correctly."
I am not sure how this can be realistically reached in C++ program which links against additional C++ libraries which may derive their own types.  GCC does this analysis by default since GCC 5 but only devirtualizes speculatively based on this information. There are -Wsuggest-final-types and -Wsuggest-final-methods which are meant precisely to give compiler information that there will be no additional derived types of the given type or method in additional libraries.

Comment 3

[[:sergesanspaille]]

Extra data points on that flag:

speedometer3 perf report: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=283ab5ce9535aadfac03555a6724d20cc401026e&newProject=try&newRevision=7c3eac38d34956077116f0799da5247a2068bafa&page=1&framework=13

the flags itself is conservative (it only applies to types with hidden visibility, ) and using -fvisibility=hidden theoretically improves its impact (in addition to having a few extra positive side effects).

We currently don't active hidden visibility by default, that could be an interesting path to explore.

Comment 4

[Mike Hommey [:glandium]]

We currently don't active hidden visibility by default, that could be an interesting path to explore.

We do on mac, and on linux, we use a force include for mostly the same effect.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Looks like chromium uses this

https://source.chromium.org/chromium/chromium/src/+/main:build/config/compiler/BUILD.gn;l=816;drc=5d6892682cfe29cb66c85cc8c07deeb27689ad3c

https://source.chromium.org/chromium/chromium/src/+/main:build/config/compiler/BUILD.gn;l=832;drc=5d6892682cfe29cb66c85cc8c07deeb27689ad3c

https://bugs.chromium.org/p/chromium/issues/detail?id=580389

It might be worth to try this out again. sp3 has changed a lot since February and Gecko has been optimized significantly.

Comment 6

[[:sergesanspaille]]

Some feedback:

1. In lto=full,cross mode, this requires the rustc flag -Zsplit-lto-unit which is not available for the rust version we use to compile shippable,

but more importantly

2. In lto=thin,cross mode, this compiles correctly but crashes at runtime (I've not investigated yet): https://treeherder.mozilla.org/jobs?repo=try&revision=90799496f8d56158ed48cc9ca3b426673d61197d&selectedTaskRun=H2khArGUQwm8SQLr2m3FLA.0