From 5a40dcfff9f62bba2b747a8d34df5b8c5674e2a1 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 12 Jul 2023 11:46:36 -0700 Subject: [PATCH 1/2] Ensure DOTNET_MaxVectorTBitwidth is interpreted as a decimal based input, not hexadecimal --- src/coreclr/inc/clrconfig.h | 3 +++ src/coreclr/inc/clrconfigvalues.h | 2 +- src/coreclr/utilcode/clrconfig.cpp | 42 ++++++++++++++++++++++++++++++ src/coreclr/vm/codeman.cpp | 3 ++- 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/src/coreclr/inc/clrconfig.h b/src/coreclr/inc/clrconfig.h index f3e22d7ded16a..88c1e09d06108 100644 --- a/src/coreclr/inc/clrconfig.h +++ b/src/coreclr/inc/clrconfig.h @@ -125,6 +125,9 @@ class CLRConfig // You own the string that's returned. static HRESULT GetConfigValue(const ConfigStringInfo & info, _Outptr_result_z_ LPWSTR * outVal); + // Reinterpret the value returned by GetConfigValue as a decimal, rather than hexadecimal input. + static DWORD ReinterpretHexAsDecimal(DWORD value); + // // Check whether an option is specified (e.g. explicitly listed) in the CLRConfig. // diff --git a/src/coreclr/inc/clrconfigvalues.h b/src/coreclr/inc/clrconfigvalues.h index d741890decf00..df5962bb9d1ce 100644 --- a/src/coreclr/inc/clrconfigvalues.h +++ b/src/coreclr/inc/clrconfigvalues.h @@ -732,7 +732,7 @@ RETAIL_CONFIG_DWORD_INFO(INTERNAL_GDBJitEmitDebugFrame, W("GDBJitEmitDebugFrame" #endif #endif -RETAIL_CONFIG_DWORD_INFO(EXTERNAL_MaxVectorTBitWidth, W("MaxVectorTBitWidth"), 0, "The maximum width, in bits, that Vector is allowed to be. A value less than 128 is treated as the system default.") +RETAIL_CONFIG_DWORD_INFO(EXTERNAL_MaxVectorTBitWidth, W("MaxVectorTBitWidth"), 0, "The maximum decimal width, in bits, that Vector is allowed to be. A value less than 128 is treated as the system default.") // // Hardware Intrinsic ISAs; keep in sync with jitconfigvalues.h diff --git a/src/coreclr/utilcode/clrconfig.cpp b/src/coreclr/utilcode/clrconfig.cpp index 8ea705a917e83..ab91e866e47c3 100644 --- a/src/coreclr/utilcode/clrconfig.cpp +++ b/src/coreclr/utilcode/clrconfig.cpp @@ -564,6 +564,48 @@ HRESULT CLRConfig::GetConfigValue(const ConfigStringInfo & info, _Outptr_result_ RETURN S_OK; } +// +// Reinterpret the value returned by GetConfigValue as a decimal, rather than hexadecimal input. +// +// Return value: +// * Value reinterpreted from a hexadecimal to decimal base +// +// Arguments: +// * value - The value to reinterpret +// +// Remarks: +// This is beneficial for a subset of configuration options, such as MaxVectorTBitWidth, +// where a decimal based input is much more intuitive to use. For example, `MaxVectorTBitWidth=256` +// is much more easily understandable than `MaxVectorTBitWidth=100` +// +// Given how GetConfigValue works, the user specifying `MaxVectorTBitWidth=256` will cause it to +// interpret that as 0x256 and therefore produce a result of 598. This function then accounts for +// that and converts it back to the intended 256 (i.e. 0x100) instead. +// +// static +DWORD CLRConfig::ReinterpretHexAsDecimal(DWORD value) +{ + unsigned result = 0; + unsigned index = 1; + + if (value == INT_MAX) + { + // default value + return value; + } + + while (value != 0) + { + unsigned digit = value % 16; + value >>= 4; + assert(digit < 10); + result += digit * index; + index *= 10; + } + + return result; +} + // // Check whether an option is specified (e.g. explicitly listed) in any location // diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 51eec2b9dae9a..9fca5dae470f0 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -1339,7 +1339,8 @@ void EEJitManager::SetCpuInfo() CORJIT_FLAGS CPUCompileFlags; // Get the maximum bitwidth of Vector, rounding down to the nearest multiple of 128-bits - uint32_t maxVectorTBitWidth = (CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_MaxVectorTBitWidth) / 128) * 128; + uint32_t maxVectorTBitWidth = CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_MaxVectorTBitWidth); + maxVectorTBitWidth = (CLRConfig::ReinterpretHexAsDecimal(maxVectorTBitWidth) * 128) / 128; #if defined(TARGET_X86) || defined(TARGET_AMD64) CPUCompileFlags.Set(InstructionSet_X86Base); From 0d202378309da95e3454a94938b2561b962c7338 Mon Sep 17 00:00:00 2001 From: Tanner Gooding Date: Wed, 12 Jul 2023 11:55:08 -0700 Subject: [PATCH 2/2] Use CLRConfig::LookupOptions::ParseIntegerAsBase10 instead --- src/coreclr/inc/clrconfig.h | 3 --- src/coreclr/inc/clrconfigvalues.h | 2 +- src/coreclr/utilcode/clrconfig.cpp | 42 ------------------------------ src/coreclr/vm/codeman.cpp | 3 +-- 4 files changed, 2 insertions(+), 48 deletions(-) diff --git a/src/coreclr/inc/clrconfig.h b/src/coreclr/inc/clrconfig.h index 88c1e09d06108..f3e22d7ded16a 100644 --- a/src/coreclr/inc/clrconfig.h +++ b/src/coreclr/inc/clrconfig.h @@ -125,9 +125,6 @@ class CLRConfig // You own the string that's returned. static HRESULT GetConfigValue(const ConfigStringInfo & info, _Outptr_result_z_ LPWSTR * outVal); - // Reinterpret the value returned by GetConfigValue as a decimal, rather than hexadecimal input. - static DWORD ReinterpretHexAsDecimal(DWORD value); - // // Check whether an option is specified (e.g. explicitly listed) in the CLRConfig. // diff --git a/src/coreclr/inc/clrconfigvalues.h b/src/coreclr/inc/clrconfigvalues.h index df5962bb9d1ce..b7357e893c3a8 100644 --- a/src/coreclr/inc/clrconfigvalues.h +++ b/src/coreclr/inc/clrconfigvalues.h @@ -732,7 +732,7 @@ RETAIL_CONFIG_DWORD_INFO(INTERNAL_GDBJitEmitDebugFrame, W("GDBJitEmitDebugFrame" #endif #endif -RETAIL_CONFIG_DWORD_INFO(EXTERNAL_MaxVectorTBitWidth, W("MaxVectorTBitWidth"), 0, "The maximum decimal width, in bits, that Vector is allowed to be. A value less than 128 is treated as the system default.") +RETAIL_CONFIG_DWORD_INFO_EX(EXTERNAL_MaxVectorTBitWidth, W("MaxVectorTBitWidth"), 0, "The maximum decimal width, in bits, that Vector is allowed to be. A value less than 128 is treated as the system default.", CLRConfig::LookupOptions::ParseIntegerAsBase10) // // Hardware Intrinsic ISAs; keep in sync with jitconfigvalues.h diff --git a/src/coreclr/utilcode/clrconfig.cpp b/src/coreclr/utilcode/clrconfig.cpp index ab91e866e47c3..8ea705a917e83 100644 --- a/src/coreclr/utilcode/clrconfig.cpp +++ b/src/coreclr/utilcode/clrconfig.cpp @@ -564,48 +564,6 @@ HRESULT CLRConfig::GetConfigValue(const ConfigStringInfo & info, _Outptr_result_ RETURN S_OK; } -// -// Reinterpret the value returned by GetConfigValue as a decimal, rather than hexadecimal input. -// -// Return value: -// * Value reinterpreted from a hexadecimal to decimal base -// -// Arguments: -// * value - The value to reinterpret -// -// Remarks: -// This is beneficial for a subset of configuration options, such as MaxVectorTBitWidth, -// where a decimal based input is much more intuitive to use. For example, `MaxVectorTBitWidth=256` -// is much more easily understandable than `MaxVectorTBitWidth=100` -// -// Given how GetConfigValue works, the user specifying `MaxVectorTBitWidth=256` will cause it to -// interpret that as 0x256 and therefore produce a result of 598. This function then accounts for -// that and converts it back to the intended 256 (i.e. 0x100) instead. -// -// static -DWORD CLRConfig::ReinterpretHexAsDecimal(DWORD value) -{ - unsigned result = 0; - unsigned index = 1; - - if (value == INT_MAX) - { - // default value - return value; - } - - while (value != 0) - { - unsigned digit = value % 16; - value >>= 4; - assert(digit < 10); - result += digit * index; - index *= 10; - } - - return result; -} - // // Check whether an option is specified (e.g. explicitly listed) in any location // diff --git a/src/coreclr/vm/codeman.cpp b/src/coreclr/vm/codeman.cpp index 9fca5dae470f0..51eec2b9dae9a 100644 --- a/src/coreclr/vm/codeman.cpp +++ b/src/coreclr/vm/codeman.cpp @@ -1339,8 +1339,7 @@ void EEJitManager::SetCpuInfo() CORJIT_FLAGS CPUCompileFlags; // Get the maximum bitwidth of Vector, rounding down to the nearest multiple of 128-bits - uint32_t maxVectorTBitWidth = CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_MaxVectorTBitWidth); - maxVectorTBitWidth = (CLRConfig::ReinterpretHexAsDecimal(maxVectorTBitWidth) * 128) / 128; + uint32_t maxVectorTBitWidth = (CLRConfig::GetConfigValue(CLRConfig::EXTERNAL_MaxVectorTBitWidth) / 128) * 128; #if defined(TARGET_X86) || defined(TARGET_AMD64) CPUCompileFlags.Set(InstructionSet_X86Base);