diff options
author | Bruce Forstall <brucefo@microsoft.com> | 2022-10-01 06:26:37 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-10-01 06:26:37 +0300 |
commit | aea856f2812c2b827c024121d987a33e3108fac3 (patch) | |
tree | 5922b903115ae03aca1c521368c6f5600908b34c /src | |
parent | 26a91ad6ac02393c6c7463500c030ec07803c5e6 (diff) |
Fix DEBUG/non-DEBUG SuperPMI asm diffs (#76470)
Recorded SPMI method contexts include configuration environment variables
such as `COMPlus_JITMinOpts` that are replayed. However, when doing
asmdiffs replays to compare a Release to a Checked compiler (non-DEBUG
to DEBUG), there may be codegen-altering configuration variables
such as JitStress that are only read and interpreted by the DEBUG
compiler. This leads to asm diffs.
Introduce a `-ignoreStoredConfig` argument to superpmi.exe, and use it
in superpmi.py when doing Checked/Release asm diffs, that pretends there
are no stored config variables. This assumes that the stored config variables
only alter JIT behavior but that they JIT will succeed with or without them.
This is also slightly more than necessary: if there is a config variable
that the Release compiler knows about, we won't use that, either. However,
we have no easy way (currently) to distinguish which variables are DEBUG
and which are both DEBUG and non-DEBUG available.
Contributes to https://github.com/dotnet/runtime/issues/76347
Diffstat (limited to 'src')
7 files changed, 35 insertions, 5 deletions
diff --git a/src/coreclr/scripts/superpmi.py b/src/coreclr/scripts/superpmi.py index ae45443620c..d4a386333a6 100644 --- a/src/coreclr/scripts/superpmi.py +++ b/src/coreclr/scripts/superpmi.py @@ -1608,6 +1608,13 @@ class SuperPMIReplayAsmDiffs: if self.coreclr_args.error_limit is not None: flags += ["-failureLimit", self.coreclr_args.error_limit] + if self.coreclr_args.diff_with_release: + # If one of the JITs is Release, ignore all the stored configuration variables. + # This isn't necessary if both are Release builds (and, in fact, it can clear variables + # that both Release compilers can interpret). However, we rarely or never compare + # two Release compilers, so this is safest. + flags += [ "-ignoreStoredConfig" ] + # Change the working directory to the Core_Root we will call SuperPMI from. # This is done to allow libcoredistools to be loaded correctly on unix # as the loadlibrary path will be relative to the current directory. diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index 23c52a3ebae..2faea5a996e 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -30,6 +30,7 @@ MethodContext::MethodContext() cr = new CompileResult(); index = -1; + ignoreStoredConfig = false; isReadyToRunCompilation = ReadyToRunCompilation::Uninitialized; } @@ -6903,6 +6904,9 @@ void MethodContext::dmpGetIntConfigValue(const Agnostic_ConfigIntInfo& key, int int MethodContext::repGetIntConfigValue(const WCHAR* name, int defaultValue) { + if (ignoreStoredConfig) + return defaultValue; + if (GetIntConfigValue == nullptr) return defaultValue; @@ -6955,6 +6959,9 @@ void MethodContext::dmpGetStringConfigValue(DWORD nameIndex, DWORD resultIndex) const WCHAR* MethodContext::repGetStringConfigValue(const WCHAR* name) { + if (ignoreStoredConfig) + return nullptr; + if (GetStringConfigValue == nullptr) return nullptr; diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h index f9769119e20..354cdb82139 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h @@ -77,9 +77,6 @@ public: MethodContext(); private: - MethodContext(HANDLE hFile); - MethodContext(unsigned char* buff, unsigned int totalLen); - void MethodInitHelper(unsigned char* buff, unsigned int totalLen); void MethodInitHelperFile(HANDLE hFile); @@ -94,6 +91,11 @@ public: ~MethodContext(); void Destroy(); + void setIgnoreStoredConfig() + { + ignoreStoredConfig = true; + } + bool Equal(MethodContext* other); unsigned int saveToFile(HANDLE hFile); unsigned int calculateFileSize(); @@ -114,8 +116,6 @@ public: void recGlobalContext(const MethodContext& other); - void dmpEnvironment(DWORD key, const Agnostic_Environment& value); - void recCompileMethod(CORINFO_METHOD_INFO* info, unsigned flags, CORINFO_OS os); void dmpCompileMethod(DWORD key, const Agnostic_CompileMethod& value); void repCompileMethod(CORINFO_METHOD_INFO* info, unsigned* flags, CORINFO_OS* os); @@ -879,6 +879,7 @@ public: CompileResult* cr; CompileResult* originalCR; int index; + bool ignoreStoredConfig; private: bool IsEnvironmentHeaderEqual(const Environment& prevEnv); diff --git a/src/coreclr/tools/superpmi/superpmi/commandline.cpp b/src/coreclr/tools/superpmi/superpmi/commandline.cpp index ca407889e87..e60a24a7a4f 100644 --- a/src/coreclr/tools/superpmi/superpmi/commandline.cpp +++ b/src/coreclr/tools/superpmi/superpmi/commandline.cpp @@ -39,6 +39,9 @@ void CommandLine::DumpHelp(const char* program) printf(" -box\n"); printf(" Break on exception thrown, such as for missing data during replay\n"); printf("\n"); + printf(" -ignoreStoredConfig\n"); + printf(" On replay, ignore any stored configuration variables. Useful for Checked/Release asm diffs.\n"); + printf("\n"); printf(" -v[erbosity] messagetypes\n"); printf(" Controls which types of messages SuperPMI logs. Specify a string of\n"); printf(" characters representing message categories to enable, where:\n"); @@ -350,6 +353,10 @@ bool CommandLine::Parse(int argc, char* argv[], /* OUT */ Options* o) { o->breakOnException = true; } + else if ((_strnicmp(&argv[i][1], "ignoreStoredConfig", 18) == 0)) + { + o->ignoreStoredConfig = true; + } else if ((_strnicmp(&argv[i][1], "verbosity", argLen) == 0)) { if (++i >= argc) diff --git a/src/coreclr/tools/superpmi/superpmi/commandline.h b/src/coreclr/tools/superpmi/superpmi/commandline.h index e801f8cb4f7..f0dc09bd161 100644 --- a/src/coreclr/tools/superpmi/superpmi/commandline.h +++ b/src/coreclr/tools/superpmi/superpmi/commandline.h @@ -22,6 +22,7 @@ public: , breakOnError(false) , breakOnAssert(false) , breakOnException(false) + , ignoreStoredConfig(false) , applyDiff(false) , parallel(false) #if !defined(USE_MSVCDIS) && defined(USE_COREDISTOOLS) @@ -59,6 +60,7 @@ public: bool breakOnError; bool breakOnAssert; bool breakOnException; + bool ignoreStoredConfig; bool applyDiff; bool parallel; // User specified to use /parallel mode. bool useCoreDisTools; // Use CoreDisTools library instead of Msvcdis diff --git a/src/coreclr/tools/superpmi/superpmi/parallelsuperpmi.cpp b/src/coreclr/tools/superpmi/superpmi/parallelsuperpmi.cpp index 610ccdf732b..228fea93934 100644 --- a/src/coreclr/tools/superpmi/superpmi/parallelsuperpmi.cpp +++ b/src/coreclr/tools/superpmi/superpmi/parallelsuperpmi.cpp @@ -487,6 +487,7 @@ char* ConstructChildProcessArgs(const CommandLine::Options& o) ADDARG_BOOL(o.breakOnError, "-boe"); ADDARG_BOOL(o.breakOnAssert, "-boa"); ADDARG_BOOL(o.breakOnException, "-box"); + ADDARG_BOOL(o.ignoreStoredConfig, "-ignoreStoredConfig"); ADDARG_BOOL(o.applyDiff, "-applyDiff"); ADDARG_STRING(o.reproName, "-reproName"); ADDARG_STRING(o.writeLogFile, "-writeLogFile"); diff --git a/src/coreclr/tools/superpmi/superpmi/superpmi.cpp b/src/coreclr/tools/superpmi/superpmi/superpmi.cpp index 7875d4ca111..c0d64df1acc 100644 --- a/src/coreclr/tools/superpmi/superpmi/superpmi.cpp +++ b/src/coreclr/tools/superpmi/superpmi/superpmi.cpp @@ -328,6 +328,11 @@ int __cdecl main(int argc, char* argv[]) return (int)SpmiResult::GeneralFailure; } + if (o.ignoreStoredConfig) + { + mc->setIgnoreStoredConfig(); + } + if (reader->IsMethodExcluded(mc)) { excludedCount++; |