Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ALT_JIT define and replace with runtime logic #44565

Merged
merged 6 commits into from
Nov 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/coreclr/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1566,7 +1566,16 @@ void MyICJI::notifyInstructionSetUsage(CORINFO_InstructionSet instructionSet, bo
DWORD MyICJI::getJitFlags(CORJIT_FLAGS* jitFlags, DWORD sizeInBytes)
{
jitInstance->mc->cr->AddCall("getJitFlags");
return jitInstance->mc->repGetJitFlags(jitFlags, sizeInBytes);
DWORD ret = jitInstance->mc->repGetJitFlags(jitFlags, sizeInBytes);
if (jitInstance->forceClearAltJitFlag)
{
jitFlags->Clear(CORJIT_FLAGS::CORJIT_FLAG_ALT_JIT);
}
else if (jitInstance->forceSetAltJitFlag)
{
jitFlags->Set(CORJIT_FLAGS::CORJIT_FLAG_ALT_JIT);
}
return ret;
}

// Runs the given function with the given parameter under an error trap
Expand Down
31 changes: 30 additions & 1 deletion src/coreclr/src/ToolBox/superpmi/superpmi/jitinstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,38 @@ JitInstance* JitInstance::InitJit(char* nameOfJit,
}

jit->forceOptions = forceOptions;

jit->options = options;

// The flag to cause the JIT to be invoked as an altjit is stored in the jit flags, not in
// the environment. If the user uses the "-jitoption force" flag to force AltJit off (it was
// probably on during collection), or to force it on, then propagate that to the jit flags.
jit->forceClearAltJitFlag = false;
jit->forceSetAltJitFlag = false;
const WCHAR* altJitFlag = jit->getForceOption(W("AltJit"));
if (altJitFlag != nullptr)
{
if (wcscmp(altJitFlag, W("")) == 0)
{
jit->forceClearAltJitFlag = true;
}
else
{
jit->forceSetAltJitFlag = true;
}
}
const WCHAR* altJitNgenFlag = jit->getForceOption(W("AltJitNgen"));
if (altJitNgenFlag != nullptr)
{
if (wcscmp(altJitNgenFlag, W("")) == 0)
{
jit->forceClearAltJitFlag = true;
}
else
{
jit->forceSetAltJitFlag = true;
}
}

jit->environment.getIntConfigValue = nullptr;
jit->environment.getStingConfigValue = nullptr;

Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/src/ToolBox/superpmi/superpmi/jitinstance.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ class JitInstance
void timeResult(CORINFO_METHOD_INFO info, unsigned flags);

public:

bool forceClearAltJitFlag;
bool forceSetAltJitFlag;

enum Result
{
RESULT_ERROR,
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/src/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,11 @@ TODO: Talk about initializing strutures before use
//
//////////////////////////////////////////////////////////////////////////////////////////////////////////

constexpr GUID JITEEVersionIdentifier = { /* a0184d06-a562-43f6-94ad-44627db87310 */
0xa0184d06,
0xa562,
0x43f6,
{0x94, 0xad, 0x44, 0x62, 0x7d, 0xb8, 0x73, 0x10}
constexpr GUID JITEEVersionIdentifier = { /* 8031aa05-4568-40fc-a0d2-d971d8edba16 */
0x8031aa05,
0x4568,
0x40fc,
{0xa0, 0xd2, 0xd9, 0x71, 0xd8, 0xed, 0xba, 0x16}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/src/inc/corjitflags.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class CORJIT_FLAGS

CORJIT_FLAG_OSR = 13, // Generate alternate method for On Stack Replacement

CORJIT_FLAG_UNUSED7 = 14,
CORJIT_FLAG_ALT_JIT = 14, // JIT should consider itself an ALT_JIT
CORJIT_FLAG_UNUSED8 = 15,
CORJIT_FLAG_UNUSED9 = 16,

Expand Down
8 changes: 2 additions & 6 deletions src/coreclr/src/jit/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ function(create_standalone_jit)

set(oneValueArgs TARGET OS ARCH)
set(multiValueArgs ADDITIONAL_DESTINATIONS)
set(options NOALTJIT)
set(options)
cmake_parse_arguments(TARGETDETAILS "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})

set(JIT_ARCH_LINK_LIBRARIES gcinfo_${TARGETDETAILS_OS}_${TARGETDETAILS_ARCH})
Expand All @@ -40,10 +40,6 @@ function(create_standalone_jit)
set_target_definitions_to_custom_os_and_arch(${ARGN})
set_target_properties(${TARGETDETAILS_TARGET} PROPERTIES IGNORE_FEATURE_MERGE_JIT_AND_ENGINE TRUE)

if (NOT TARGETDETAILS_NOALTJIT)
target_compile_definitions(${TARGETDETAILS_TARGET} PRIVATE ALT_JIT)
endif (NOT TARGETDETAILS_NOALTJIT)

target_compile_definitions(${TARGETDETAILS_TARGET} PRIVATE FEATURE_NO_HOST)
target_compile_definitions(${TARGETDETAILS_TARGET} PRIVATE SELF_NO_HOST)
if(FEATURE_READYTORUN)
Expand Down Expand Up @@ -461,7 +457,7 @@ else()
set(TARGET_OS_NAME win)
endif()

create_standalone_jit(TARGET clrjit OS ${TARGET_OS_NAME} ARCH ${ARCH_TARGET_NAME} NOALTJIT ADDITIONAL_DESTINATIONS sharedFramework)
create_standalone_jit(TARGET clrjit OS ${TARGET_OS_NAME} ARCH ${ARCH_TARGET_NAME} ADDITIONAL_DESTINATIONS sharedFramework)

# Enable profile guided optimization
add_pgo(clrjit)
Expand Down
5 changes: 5 additions & 0 deletions src/coreclr/src/jit/codegencommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2211,6 +2211,11 @@ void CodeGen::genGenerateMachineCode()
{
printf("; discarded IBC profile data due to mismatch in ILSize\n");
}

if (compiler->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_ALT_JIT))
{
printf("; invoked as altjit\n");
}
}
#endif // DEBUG

Expand Down
50 changes: 21 additions & 29 deletions src/coreclr/src/jit/compiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,9 @@ unsigned Compiler::jitTotalMethodCompiled = 0;
LONG Compiler::jitNestingLevel = 0;
#endif // defined(DEBUG)

#ifdef ALT_JIT
// static
bool Compiler::s_pAltJitExcludeAssembliesListInitialized = false;
AssemblyNamesList2* Compiler::s_pAltJitExcludeAssembliesList = nullptr;
#endif // ALT_JIT

#ifdef DEBUG
// static
Expand Down Expand Up @@ -1353,13 +1351,11 @@ void Compiler::compStartup()
/* static */
void Compiler::compShutdown()
{
#ifdef ALT_JIT
if (s_pAltJitExcludeAssembliesList != nullptr)
{
s_pAltJitExcludeAssembliesList->~AssemblyNamesList2(); // call the destructor
s_pAltJitExcludeAssembliesList = nullptr;
}
#endif // ALT_JIT

#ifdef DEBUG
if (s_pJitDisasmIncludeAssembliesList != nullptr)
Expand Down Expand Up @@ -2582,18 +2578,19 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
pfAltJit = &JitConfig.AltJit();
}

#ifdef ALT_JIT
if (pfAltJit->contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_ALT_JIT))
{
opts.altJit = true;
}
if (pfAltJit->contains(info.compMethodName, info.compClassName, &info.compMethodInfo->args))
{
opts.altJit = true;
}

unsigned altJitLimit = ReinterpretHexAsDecimal(JitConfig.AltJitLimit());
if (altJitLimit > 0 && Compiler::jitTotalMethodCompiled >= altJitLimit)
{
opts.altJit = false;
unsigned altJitLimit = ReinterpretHexAsDecimal(JitConfig.AltJitLimit());
if (altJitLimit > 0 && Compiler::jitTotalMethodCompiled >= altJitLimit)
{
opts.altJit = false;
}
}
#endif // ALT_JIT

#else // !DEBUG

Expand All @@ -2607,20 +2604,20 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
altJitVal = JitConfig.AltJit().list();
}

#ifdef ALT_JIT
// In release mode, you either get all methods or no methods. You must use "*" as the parameter, or we ignore it.
// You don't get to give a regular expression of methods to match.
// (Partially, this is because we haven't computed and stored the method and class name except in debug, and it
// might be expensive to do so.)
if ((altJitVal != nullptr) && (strcmp(altJitVal, "*") == 0))
if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_ALT_JIT))
{
opts.altJit = true;
// In release mode, you either get all methods or no methods. You must use "*" as the parameter, or we ignore
// it. You don't get to give a regular expression of methods to match.
// (Partially, this is because we haven't computed and stored the method and class name except in debug, and it
// might be expensive to do so.)
if ((altJitVal != nullptr) && (strcmp(altJitVal, "*") == 0))
{
opts.altJit = true;
}
}
#endif // ALT_JIT

#endif // !DEBUG

#ifdef ALT_JIT
// Take care of COMPlus_AltJitExcludeAssemblies.
if (opts.altJit)
{
Expand Down Expand Up @@ -2652,7 +2649,6 @@ void Compiler::compInitOptions(JitFlags* jitFlags)
}
}
}
#endif // ALT_JIT

#ifdef DEBUG

Expand Down Expand Up @@ -5920,14 +5916,12 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE classPtr,

compInitOptions(compileFlags);

#ifdef ALT_JIT
if (!compIsForInlining() && !opts.altJit)
if (!compIsForInlining() && !opts.altJit && opts.jitFlags->IsSet(JitFlags::JIT_FLAG_ALT_JIT))
{
// We're an altjit, but the COMPlus_AltJit configuration did not say to compile this method,
// so skip it.
return CORJIT_SKIPPED;
}
#endif // ALT_JIT

#ifdef DEBUG

Expand Down Expand Up @@ -6217,14 +6211,12 @@ int Compiler::compCompileHelper(CORINFO_MODULE_HANDLE classPtr,
return CORJIT_SKIPPED;
}

#ifdef ALT_JIT
#ifdef DEBUG
if (JitConfig.RunAltJitCode() == 0)
if (opts.jitFlags->IsSet(JitFlags::JIT_FLAG_ALT_JIT) && JitConfig.RunAltJitCode() == 0)
{
return CORJIT_SKIPPED;
}
#endif // DEBUG
#endif // ALT_JIT
}

/* Success! */
Expand Down
2 changes: 0 additions & 2 deletions src/coreclr/src/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -9011,10 +9011,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
#endif
} opts;

#ifdef ALT_JIT
static bool s_pAltJitExcludeAssembliesListInitialized;
static AssemblyNamesList2* s_pAltJitExcludeAssembliesList;
#endif // ALT_JIT

#ifdef DEBUG
static bool s_pJitDisasmIncludeAssembliesListInitialized;
Expand Down
45 changes: 23 additions & 22 deletions src/coreclr/src/jit/error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,15 @@ void noWayAssertBodyConditional(
}
}

#if defined(ALT_JIT)

/*****************************************************************************/
void notYetImplemented(const char* msg, const char* filename, unsigned line)
{
Compiler* pCompiler = JitTls::GetCompiler();
if ((pCompiler == nullptr) || (pCompiler->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_ALT_JIT)))
{
NOWAY_MSG_FILE_AND_LINE(msg, filename, line);
return;
}
#if FUNC_INFO_LOGGING
#ifdef DEBUG
LogEnv* env = JitTls::GetLogEnv();
Expand Down Expand Up @@ -171,12 +175,8 @@ void notYetImplemented(const char* msg, const char* filename, unsigned line)
#endif // FUNC_INFO_LOGGING

#ifdef DEBUG
Compiler* pCompiler = JitTls::GetCompiler();
if (pCompiler != nullptr)
{
// Assume we're within a compFunctionTrace boundary, which might not be true.
pCompiler->compFunctionTraceEnd(nullptr, 0, true);
}
// Assume we're within a compFunctionTrace boundary, which might not be true.
pCompiler->compFunctionTraceEnd(nullptr, 0, true);
#endif // DEBUG

DWORD value = JitConfig.AltJitAssertOnNYI();
Expand All @@ -203,8 +203,6 @@ void notYetImplemented(const char* msg, const char* filename, unsigned line)
}
}

#endif // #if defined(ALT_JIT)

/*****************************************************************************/
LONG __JITfilter(PEXCEPTION_POINTERS pExceptionPointers, LPVOID lpvParam)
{
Expand Down Expand Up @@ -305,20 +303,23 @@ extern "C" void __cdecl assertAbort(const char* why, const char* file, unsigned
DebugBreak();
}

#ifdef ALT_JIT
// If we hit an assert, and we got here, it's either because the user hit "ignore" on the
// dialog pop-up, or they set COMPlus_ContinueOnAssert=1 to not emit a pop-up, but just continue.
// If we're an altjit, we have two options: (1) silently continue, as a normal JIT would, probably
// leading to additional asserts, or (2) tell the VM that the AltJit wants to skip this function,
// thus falling back to the fallback JIT. Setting COMPlus_AltJitSkipOnAssert=1 chooses this "skip"
// to the fallback JIT behavior. This is useful when doing ASM diffs, where we only want to see
// the first assert for any function, but we don't want to kill the whole ngen process on the
// first assert (which would happen if you used COMPlus_NoGuiOnAssert=1 for example).
if (JitConfig.AltJitSkipOnAssert() != 0)
Compiler* comp = JitTls::GetCompiler();

if (comp != nullptr && comp->opts.jitFlags->IsSet(JitFlags::JIT_FLAG_ALT_JIT))
{
fatal(CORJIT_SKIPPED);
// If we hit an assert, and we got here, it's either because the user hit "ignore" on the
// dialog pop-up, or they set COMPlus_ContinueOnAssert=1 to not emit a pop-up, but just continue.
// If we're an altjit, we have two options: (1) silently continue, as a normal JIT would, probably
// leading to additional asserts, or (2) tell the VM that the AltJit wants to skip this function,
// thus falling back to the fallback JIT. Setting COMPlus_AltJitSkipOnAssert=1 chooses this "skip"
// to the fallback JIT behavior. This is useful when doing ASM diffs, where we only want to see
// the first assert for any function, but we don't want to kill the whole ngen process on the
// first assert (which would happen if you used COMPlus_NoGuiOnAssert=1 for example).
if (JitConfig.AltJitSkipOnAssert() != 0)
{
fatal(CORJIT_SKIPPED);
}
}
#endif
}

/*********************************************************************/
Expand Down
10 changes: 2 additions & 8 deletions src/coreclr/src/jit/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ extern void RecordNowayAssertGlobal(const char* filename, unsigned line, const c
#define unreached() noWayAssertBody("unreached", __FILE__, __LINE__)

#define NOWAY_MSG(msg) noWayAssertBodyConditional(msg, __FILE__, __LINE__)
#define NOWAY_MSG_FILE_AND_LINE(msg, file, line) noWayAssertBodyConditional(msg, file, line)

// IMPL_LIMITATION is called when we encounter valid IL that is not
// supported by our current implementation because of various
Expand Down Expand Up @@ -153,24 +154,17 @@ extern void RecordNowayAssertGlobal(const char* filename, unsigned line, const c
#define unreached() noWayAssertBody()

#define NOWAY_MSG(msg) noWayAssertBodyConditional(NOWAY_ASSERT_BODY_ARGUMENTS)
#define NOWAY_MSG_FILE_AND_LINE(msg, file, line) noWayAssertBodyConditional(NOWAY_ASSERT_BODY_ARGUMENTS)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess file and line are ignored here, in favor of __FILE__, __LINE__

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Well, sortof. This is the release portion of the ifdef, where FILE and LINE aren't used at all. The goal is to match the overall behavior of what existed before.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In particular, non-altjits don't put in file/line number stuff, but chk/dbg will.


#endif // !DEBUG


#if 1 // All platforms currently enable NYI; this should be a tighter condition to exclude some platforms from NYI

#if defined(ALT_JIT)

// This can return based on Config flag/Debugger
extern void notYetImplemented(const char* msg, const char* file, unsigned line);
#define NYIRAW(msg) notYetImplemented(msg, __FILE__, __LINE__)

#else // !defined(ALT_JIT)

#define NYIRAW(msg) NOWAY_MSG(msg)

#endif // !defined(ALT_JIT)

#define NYI(msg) NYIRAW("NYI: " msg)
davidwrighton marked this conversation as resolved.
Show resolved Hide resolved
#define NYI_IF(cond, msg) if (cond) NYIRAW("NYI: " msg)

Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/src/jit/jit.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,6 @@
#endif
#endif

#if (defined(ALT_JIT) && (defined(UNIX_AMD64_ABI) || defined(UNIX_X86_ABI)) && !defined(TARGET_UNIX))
// If we are building an ALT_JIT targeting Unix, override the TARGET_<os> to TARGET_UNIX
#undef TARGET_WINDOWS
#define TARGET_UNIX
#endif

// --------------------------------------------------------------------------------
// IMAGE_FILE_MACHINE_TARGET
// --------------------------------------------------------------------------------
Expand Down
Loading