From 212a6baa708f40e25860581557b3dda717b709d9 Mon Sep 17 00:00:00 2001 From: Sergey Andreenko Date: Thu, 28 Sep 2017 10:37:15 -0700 Subject: [PATCH] Add the capability to force JIT options on the SPMI command line. (#14209) expand the grammar for jit{,2}option to -jitoption [force] key=value --- src/ToolBox/superpmi/superpmi/commandline.cpp | 112 +++++++++++++-------- src/ToolBox/superpmi/superpmi/commandline.h | 6 ++ src/ToolBox/superpmi/superpmi/jithost.cpp | 39 +++++-- src/ToolBox/superpmi/superpmi/jitinstance.cpp | 14 ++- src/ToolBox/superpmi/superpmi/jitinstance.h | 5 +- src/ToolBox/superpmi/superpmi/parallelsuperpmi.cpp | 47 +++++---- src/ToolBox/superpmi/superpmi/superpmi.cpp | 4 +- 7 files changed, 155 insertions(+), 72 deletions(-) diff --git a/src/ToolBox/superpmi/superpmi/commandline.cpp b/src/ToolBox/superpmi/superpmi/commandline.cpp index 47ba094..5394cf2 100644 --- a/src/ToolBox/superpmi/superpmi/commandline.cpp +++ b/src/ToolBox/superpmi/superpmi/commandline.cpp @@ -119,11 +119,13 @@ void CommandLine::DumpHelp(const char* program) printf(" Ignored: neither MSVCDIS nor CoreDisTools is available.\n"); #endif // USE_COREDISTOOLS printf("\n"); - printf(" -jitoption key=value\n"); - printf(" Set the JIT option named \"key\" to \"value\" for JIT 1. NOTE: do not use a \"COMPlus_\" prefix!\n"); + printf(" -jitoption [force] key=value\n"); + printf(" Set the JIT option named \"key\" to \"value\" for JIT 1 if the option was not set."); + printf(" With optional force flag overwrites the existing value if it was already set. NOTE: do not use a \"COMPlus_\" prefix!\n"); printf("\n"); - printf(" -jit2option key=value\n"); - printf(" Set the JIT option named \"key\" to \"value\" for JIT 2. NOTE: do not use a \"COMPlus_\" prefix!\n"); + printf(" -jit2option [force] key=value\n"); + printf(" Set the JIT option named \"key\" to \"value\" for JIT 2 if the option was not set."); + printf(" With optional force flag overwrites the existing value if it was already set. NOTE: do not use a \"COMPlus_\" prefix!\n"); printf("\n"); printf("Inputs are case sensitive.\n"); printf("\n"); @@ -163,10 +165,6 @@ static bool ParseJitOption(const char* optionString, wchar_t** key, wchar_t **va tempKey[i] = '\0'; const char* tempVal = &optionString[i + 1]; - if (tempVal[0] == '\0') - { - return false; - } const unsigned keyLen = i; wchar_t* keyBuf = new wchar_t[keyLen + 1]; @@ -522,51 +520,22 @@ bool CommandLine::Parse(int argc, char* argv[], /* OUT */ Options* o) return false; } } - else if ((_strnicmp(&argv[i][1], "jitoption", argLen) == 0)) + else if (_strnicmp(&argv[i][1], "jitoption", argLen) == 0) { i++; - - wchar_t *key, *value; - if ((i >= argc) || !ParseJitOption(argv[i], &key, &value)) + if (!AddJitOption(i, argc, argv, &o->jitOptions, &o->forceJitOptions)) { - DumpHelp(argv[0]); return false; } - - if (o->jitOptions == nullptr) - { - o->jitOptions = new LightWeightMap(); - } - - DWORD keyIndex = (DWORD)o->jitOptions->AddBuffer((unsigned char*)key, sizeof(wchar_t) * ((unsigned int)wcslen(key) + 1)); - DWORD valueIndex = (DWORD)o->jitOptions->AddBuffer((unsigned char*)value, sizeof(wchar_t) * ((unsigned int)wcslen(value) + 1)); - o->jitOptions->Add(keyIndex, valueIndex); - - delete[] key; - delete[] value; } - else if ((_strnicmp(&argv[i][1], "jit2option", argLen) == 0)) + else if (_strnicmp(&argv[i][1], "jit2option", argLen) == 0) { - i++; - - wchar_t *key, *value; - if ((i >= argc) || !ParseJitOption(argv[i], &key, &value)) + i++; + if (!AddJitOption(i, argc, argv, &o->jit2Options, &o->forceJit2Options)) { - DumpHelp(argv[0]); return false; } - if (o->jit2Options == nullptr) - { - o->jit2Options = new LightWeightMap(); - } - - DWORD keyIndex = (DWORD)o->jit2Options->AddBuffer((unsigned char*)key, sizeof(wchar_t) * ((unsigned int)wcslen(key) + 1)); - DWORD valueIndex = (DWORD)o->jit2Options->AddBuffer((unsigned char*)value, sizeof(wchar_t) * ((unsigned int)wcslen(value) + 1)); - o->jit2Options->Add(keyIndex, valueIndex); - - delete[] key; - delete[] value; } else { @@ -637,3 +606,62 @@ bool CommandLine::Parse(int argc, char* argv[], /* OUT */ Options* o) } return true; } + +//------------------------------------------------------------- +// AddJitOption: Parse the value that was passed with -jitOption flag. +// +// Arguments: +// currArgument - current argument number. Points to the token next to -jitOption. After the function +// points to the last parsed token +// argc - number of all arguments +// argv - arguments as array of char* +// pJitOptions - a jit options map, that sets the option, if it was not already set. +// pForceJitOptions - a jit options map, that forces the option even if it was already set. +// +// Returns: +// False if an error occurred, true if the option was parsed and added. +bool CommandLine::AddJitOption(int& currArgument, int argc, char* argv[], LightWeightMap** pJitOptions, LightWeightMap** pForceJitOptions) +{ + if (currArgument >= argc) + { + DumpHelp(argv[0]); + return false; + } + + LightWeightMap* targetjitOptions = nullptr; + + + if (_strnicmp(argv[currArgument], "force", strlen(argv[currArgument])) == 0) + { + if (*pForceJitOptions == nullptr) + { + *pForceJitOptions = new LightWeightMap(); + } + targetjitOptions = *pForceJitOptions; + currArgument++; + } + else + { + if (*pJitOptions == nullptr) + { + *pJitOptions = new LightWeightMap(); + } + targetjitOptions = *pJitOptions; + } + + wchar_t* key; + wchar_t* value; + if ((currArgument >= argc) || !ParseJitOption(argv[currArgument], &key, &value)) + { + DumpHelp(argv[0]); + return false; + } + + DWORD keyIndex = (DWORD)targetjitOptions->AddBuffer((unsigned char*)key, sizeof(wchar_t) * ((unsigned int)wcslen(key) + 1)); + DWORD valueIndex = (DWORD)targetjitOptions->AddBuffer((unsigned char*)value, sizeof(wchar_t) * ((unsigned int)wcslen(value) + 1)); + targetjitOptions->Add(keyIndex, valueIndex); + + delete[] key; + delete[] value; + return true; +} diff --git a/src/ToolBox/superpmi/superpmi/commandline.h b/src/ToolBox/superpmi/superpmi/commandline.h index 0881f08..cf9d79c 100644 --- a/src/ToolBox/superpmi/superpmi/commandline.h +++ b/src/ToolBox/superpmi/superpmi/commandline.h @@ -42,6 +42,8 @@ public: , compileList(nullptr) , offset(-1) , increment(-1) + , forceJitOptions(nullptr) + , forceJit2Options(nullptr) , jitOptions(nullptr) , jit2Options(nullptr) { @@ -69,12 +71,16 @@ public: char* compileList; int offset; int increment; + LightWeightMap* forceJitOptions; + LightWeightMap* forceJit2Options; LightWeightMap* jitOptions; LightWeightMap* jit2Options; }; static bool Parse(int argc, char* argv[], /* OUT */ Options* o); + static bool AddJitOption(int& currArgument, int argc, char* argv[], LightWeightMap** pJitOptions, LightWeightMap** pForceJitOptions); + private: static void DumpHelp(const char* program); }; diff --git a/src/ToolBox/superpmi/superpmi/jithost.cpp b/src/ToolBox/superpmi/superpmi/jithost.cpp index a515d51..f92c6f4 100644 --- a/src/ToolBox/superpmi/superpmi/jithost.cpp +++ b/src/ToolBox/superpmi/superpmi/jithost.cpp @@ -64,7 +64,23 @@ void JitHost::freeMemory(void* block, bool usePageAllocator) int JitHost::getIntConfigValue(const wchar_t* key, int defaultValue) { jitInstance.mc->cr->AddCall("getIntConfigValue"); - int result = jitInstance.mc->repGetIntConfigValue(key, defaultValue); + + int result = defaultValue; + + const wchar_t* forceValue = jitInstance.getForceOption(key); + + if (forceValue != nullptr) + { + wchar_t* endPtr; + result = static_cast(wcstoul(forceValue, &endPtr, 16)); + bool succeeded = (errno != ERANGE) && (endPtr != forceValue); + if (succeeded) + { + return result; + } + } + + result = jitInstance.mc->repGetIntConfigValue(key, defaultValue); if (result != defaultValue) { @@ -80,12 +96,12 @@ int JitHost::getIntConfigValue(const wchar_t* key, int defaultValue) // If the result is the default value, probe the JIT options and then the environment. If a value is found, parse // it as a hex integer. - wchar_t* endPtr; - bool succeeded; - const wchar_t* value = jitInstance.getOption(key); + + bool succeeded; if (value != nullptr) { + wchar_t* endPtr; result = static_cast(wcstoul(value, &endPtr, 16)); succeeded = (errno != ERANGE) && (endPtr != value); } @@ -96,7 +112,7 @@ int JitHost::getIntConfigValue(const wchar_t* key, int defaultValue) { return defaultValue; } - + wchar_t* endPtr; result = static_cast(wcstoul(complus, &endPtr, 16)); succeeded = (errno != ERANGE) && (endPtr != complus); jitInstance.freeLongLivedArray(complus); @@ -108,9 +124,18 @@ int JitHost::getIntConfigValue(const wchar_t* key, int defaultValue) const wchar_t* JitHost::getStringConfigValue(const wchar_t* key) { jitInstance.mc->cr->AddCall("getStringConfigValue"); - const wchar_t* result = jitInstance.mc->repGetStringConfigValue(key); - // If the result is the default value, probe the JIT options and then the environment. + const wchar_t* result = nullptr; + + // First check the force options, then mc value. If value is not presented there, probe the JIT options and then the environment. + + result = jitInstance.getForceOption(key); + + if (result == nullptr) + { + result = jitInstance.mc->repGetStringConfigValue(key); + } + if (result == nullptr) { result = jitInstance.getOption(key); diff --git a/src/ToolBox/superpmi/superpmi/jitinstance.cpp b/src/ToolBox/superpmi/superpmi/jitinstance.cpp index 2d84cfd..37cb54d 100644 --- a/src/ToolBox/superpmi/superpmi/jitinstance.cpp +++ b/src/ToolBox/superpmi/superpmi/jitinstance.cpp @@ -12,7 +12,7 @@ #include "errorhandling.h" #include "spmiutil.h" -JitInstance* JitInstance::InitJit(char* nameOfJit, bool breakOnAssert, SimpleTimer* st1, MethodContext* firstContext, LightWeightMap* options) +JitInstance* JitInstance::InitJit(char* nameOfJit, bool breakOnAssert, SimpleTimer* st1, MethodContext* firstContext, LightWeightMap* forceOptions, LightWeightMap* options) { JitInstance* jit = new JitInstance(); if (jit == nullptr) @@ -21,6 +21,8 @@ JitInstance* JitInstance::InitJit(char* nameOfJit, bool breakOnAssert, SimpleTim return nullptr; } + jit->forceOptions = forceOptions; + jit->options = options; if (st1 != nullptr) @@ -399,8 +401,18 @@ void JitInstance::timeResult(CORINFO_METHOD_INFO info, unsigned flags) /*-------------------------- Misc ---------------------------------------*/ +const wchar_t* JitInstance::getForceOption(const wchar_t* key) +{ + return getOption(key, forceOptions); +} + const wchar_t* JitInstance::getOption(const wchar_t* key) { + return getOption(key, options); +} + +const wchar_t* JitInstance::getOption(const wchar_t* key, LightWeightMap* options) +{ if (options == nullptr) { return nullptr; diff --git a/src/ToolBox/superpmi/superpmi/jitinstance.h b/src/ToolBox/superpmi/superpmi/jitinstance.h index 0baf401..1172b9c 100644 --- a/src/ToolBox/superpmi/superpmi/jitinstance.h +++ b/src/ToolBox/superpmi/superpmi/jitinstance.h @@ -25,6 +25,7 @@ private: ICorJitInfo* icji; SimpleTimer stj; + LightWeightMap* forceOptions; LightWeightMap* options; JitInstance(){}; @@ -43,7 +44,7 @@ public: ICorJitCompiler* pJitInstance; // Allocate and initialize the jit provided - static JitInstance* InitJit(char* nameOfJit, bool breakOnAssert, SimpleTimer* st1, MethodContext* firstContext, LightWeightMap* options); + static JitInstance* InitJit(char* nameOfJit, bool breakOnAssert, SimpleTimer* st1, MethodContext* firstContext, LightWeightMap* forceOptions, LightWeightMap* options); HRESULT StartUp(char* PathToJit, bool copyJit, bool breakOnDebugBreakorAV, MethodContext* firstContext); bool reLoad(MethodContext* firstContext); @@ -52,7 +53,9 @@ public: Result CompileMethod(MethodContext* MethodToCompile, int mcIndex, bool collectThroughput); + const wchar_t* getForceOption(const wchar_t* key); const wchar_t* getOption(const wchar_t* key); + const wchar_t* getOption(const wchar_t* key, LightWeightMap* options); void* allocateArray(ULONG size); void* allocateLongLivedArray(ULONG size); diff --git a/src/ToolBox/superpmi/superpmi/parallelsuperpmi.cpp b/src/ToolBox/superpmi/superpmi/parallelsuperpmi.cpp index 41b27de..f4c305d 100644 --- a/src/ToolBox/superpmi/superpmi/parallelsuperpmi.cpp +++ b/src/ToolBox/superpmi/superpmi/parallelsuperpmi.cpp @@ -323,8 +323,31 @@ void MergeWorkerMCLs(char* mclFilename, char** arrWorkerMCLPath, int workerCount LogError("Unable to write to MCL file %s.", mclFilename); } -// From the arguments that we parsed, construct the arguments to pass to the child processes. #define MAX_CMDLINE_SIZE 0x1000 // 4 KB + +//------------------------------------------------------------- +// addJitOptionArgument: Writes jitOption arguments to the argument string for a child spmi process. +// +// Arguments: +// jitOptions - map with options +// bytesWritten - size of the argument string in bytes +// spmiArgs - pointer to the argument string +// optionName - the jitOption name, can include [force] flag. +// +void addJitOptionArgument(LightWeightMap* jitOptions, int &bytesWritten, char * spmiArgs, const char* optionName) +{ + if (jitOptions != nullptr) + { + for (unsigned i = 0; i < jitOptions->GetCount(); i++) + { + wchar_t* key = (wchar_t*)jitOptions->GetBuffer(jitOptions->GetKey(i)); + wchar_t* value = (wchar_t*)jitOptions->GetBuffer(jitOptions->GetItem(i)); + bytesWritten += sprintf_s(spmiArgs + bytesWritten, MAX_CMDLINE_SIZE - bytesWritten, " -%s %S=%S", optionName, key, value); + } + } +} + +// From the arguments that we parsed, construct the arguments to pass to the child processes. char* ConstructChildProcessArgs(const CommandLine::Options& o) { int bytesWritten = 0; @@ -361,25 +384,11 @@ char* ConstructChildProcessArgs(const CommandLine::Options& o) ADDARG_STRING(o.targetArchitecture, "-target"); ADDARG_STRING(o.compileList, "-compile"); - if (o.jitOptions != nullptr) - { - for (unsigned i = 0; i < o.jitOptions->GetCount(); i++) - { - wchar_t* key = (wchar_t*)o.jitOptions->GetBuffer(o.jitOptions->GetKey(i)); - wchar_t* value = (wchar_t*)o.jitOptions->GetBuffer(o.jitOptions->GetItem(i)); - bytesWritten += sprintf_s(spmiArgs + bytesWritten, MAX_CMDLINE_SIZE - bytesWritten, " -jitoption %S=%S", key, value); - } - } + addJitOptionArgument(o.forceJitOptions, bytesWritten, spmiArgs, "jitoption force"); + addJitOptionArgument(o.forceJit2Options, bytesWritten, spmiArgs, "jit2option force"); - if (o.jit2Options != nullptr) - { - for (unsigned i = 0; i < o.jit2Options->GetCount(); i++) - { - wchar_t* key = (wchar_t*)o.jit2Options->GetBuffer(o.jit2Options->GetKey(i)); - wchar_t* value = (wchar_t*)o.jit2Options->GetBuffer(o.jit2Options->GetItem(i)); - bytesWritten += sprintf_s(spmiArgs + bytesWritten, MAX_CMDLINE_SIZE - bytesWritten, " -jit2option %S=%S", key, value); - } - } + addJitOptionArgument(o.jitOptions, bytesWritten, spmiArgs, "jitoption"); + addJitOptionArgument(o.jit2Options, bytesWritten, spmiArgs, "jit2option"); ADDSTRING(o.nameOfJit); ADDSTRING(o.nameOfJit2); diff --git a/src/ToolBox/superpmi/superpmi/superpmi.cpp b/src/ToolBox/superpmi/superpmi/superpmi.cpp index 9c44222..89f1d58 100644 --- a/src/ToolBox/superpmi/superpmi/superpmi.cpp +++ b/src/ToolBox/superpmi/superpmi/superpmi.cpp @@ -287,7 +287,7 @@ int __cdecl main(int argc, char* argv[]) { SimpleTimer stInitJit; - jit = JitInstance::InitJit(o.nameOfJit, o.breakOnAssert, &stInitJit, mc, o.jitOptions); + jit = JitInstance::InitJit(o.nameOfJit, o.breakOnAssert, &stInitJit, mc, o.forceJitOptions, o.jitOptions); if (jit == nullptr) { // InitJit already printed a failure message @@ -296,7 +296,7 @@ int __cdecl main(int argc, char* argv[]) if (o.nameOfJit2 != nullptr) { - jit2 = JitInstance::InitJit(o.nameOfJit2, o.breakOnAssert, &stInitJit, mc, o.jit2Options); + jit2 = JitInstance::InitJit(o.nameOfJit2, o.breakOnAssert, &stInitJit, mc, o.forceJit2Options, o.jit2Options); if (jit2 == nullptr) { // InitJit already printed a failure message -- 2.7.4