From 432369ae8476a0ad7dee187be86bc1412afb6a4c Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Thu, 11 Jul 2019 22:00:07 -0400 Subject: [PATCH] [kernel] Define param order between compilers Fix issue in clang where args were inverted last to first due to the way c++ implements function calls. The function make_tuple, being a function, has undefined ordering of resolution of the values of its parameters `Ps(init)` and would vary between compilers. Namely clang would resolve them in order and msvc would resolve them in reverse order. This normally would not matter except for the fact that init maintains a mutable state which is affected my the order of operations: init.ordinal is a counter and also defines where in memory a value is stored. The C++ standard doesn't define an order of resolution of parameters in a function but it will define an order in a brace-initializer. Switching make_tuple for a brace-initializer enforces an order which is the same between all compilers (tested gcc, clang and msvc). Prior code was written to decrement ordinal due to the reverse traversal. This has been switched to incrementing thanks to the in-order traversal. --- src/xenia/kernel/util/shim_utils.h | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/xenia/kernel/util/shim_utils.h b/src/xenia/kernel/util/shim_utils.h index fcfc90d4d..126358b17 100644 --- a/src/xenia/kernel/util/shim_utils.h +++ b/src/xenia/kernel/util/shim_utils.h @@ -148,7 +148,7 @@ class Param { protected: Param() : ordinal_(-1) {} - explicit Param(Init& init) : ordinal_(--init.ordinal) {} + explicit Param(Init& init) : ordinal_(init.ordinal++) {} template void LoadValue(Init& init, V* out_value) { @@ -519,10 +519,13 @@ xe::cpu::Export* RegisterExport(R (*fn)(Ps&...), const char* name, ++export_entry->function_data.call_count; Param::Init init = { ppc_context, - sizeof...(Ps), 0, }; - auto params = std::make_tuple(Ps(init)...); + // Using braces initializer instead of make_tuple because braces + // enforce execution order across compilers. + // The make_tuple order is undefined per the C++ standard and + // cause inconsitencies between msvc and clang. + std::tuple params = {Ps(init)...}; if (export_entry->tags & xe::cpu::ExportTag::kLog && (!(export_entry->tags & xe::cpu::ExportTag::kHighFrequency) || cvars::log_high_frequency_kernel_calls)) { @@ -554,9 +557,13 @@ xe::cpu::Export* RegisterExport(void (*fn)(Ps&...), const char* name, ++export_entry->function_data.call_count; Param::Init init = { ppc_context, - sizeof...(Ps), + 0, }; - auto params = std::make_tuple(Ps(init)...); + // Using braces initializer instead of make_tuple because braces + // enforce execution order across compilers. + // The make_tuple order is undefined per the C++ standard and + // cause inconsitencies between msvc and clang. + std::tuple params = {Ps(init)...}; if (export_entry->tags & xe::cpu::ExportTag::kLog && (!(export_entry->tags & xe::cpu::ExportTag::kHighFrequency) || cvars::log_high_frequency_kernel_calls)) {