Open Bug 1517816 Opened 5 years ago Updated 26 days ago

Enable -fwhole-program-vtables for clang builds

Categories

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

enhancement

Tracking

(Not tracked)

People

(Reporter: jewilde, Assigned: sergesanspaille)

References

Details

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
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
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.
Status: NEW → ASSIGNED
Assignee: jewilde → nobody
Status: ASSIGNED → NEW
Severity: normal → S3

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.

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.

Assignee: nobody → sguelton
Flags: needinfo?(sguelton)

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

  1. 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
You need to log in before you can comment on or make changes to this bug.