diff options
author | Richard Henderson <richard.henderson@linaro.org> | 2020-07-26 15:39:53 -0700 |
---|---|---|
committer | Richard Henderson <richard.henderson@linaro.org> | 2020-09-03 13:13:58 -0700 |
commit | 6b8b622e87e2cb4b22113f2bdebf18c78f5905ee (patch) | |
tree | 3e567b49c46d147ec402e8b8596a53cba2326d84 /tcg | |
parent | 3dd23a4fb8fd72d2220a90a809f213999ffe7f3a (diff) |
cputlb: Make store_helper less fragile to compiler optimizations
This has no functional change.
The current function structure is:
inline QEMU_ALWAYSINLINE
store_memop() {
switch () {
...
default:
qemu_build_not_reached();
}
}
inline QEMU_ALWAYSINLINE
store_helper() {
...
if (span_two_pages_or_io) {
...
helper_ret_stb_mmu();
}
store_memop();
}
helper_ret_stb_mmu() {
store_helper();
}
Whereas GCC will generate an error at compile-time when an always_inline
function is not inlined, Clang does not. Nor does Clang prioritize the
inlining of always_inline functions. Both of these are arguably bugs.
Both `store_memop` and `store_helper` need to be inlined and allow
constant propogations to eliminate the `qemu_build_not_reached` call.
However, if the compiler instead chooses to inline helper_ret_stb_mmu
into store_helper, then store_helper is now self-recursive and the
compiler is no longer able to propagate the constant in the same way.
This does not produce at current QEMU head, but was reproducible
at v4.2.0 with `clang-10 -O2 -fexperimental-new-pass-manager`.
The inline recursion problem can be fixed solely by marking
helper_ret_stb_mmu as noinline, so the compiler does not make an
incorrect decision about which functions to inline.
In addition, extract store_helper_unaligned as a noinline subroutine
that can be shared by all of the helpers. This saves about 6k code
size in an optimized x86_64 build.
Reported-by: Shu-Chun Weng <scw@google.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Diffstat (limited to 'tcg')
0 files changed, 0 insertions, 0 deletions