-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Conversation
@jkotas PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo a few comments.
src/vm/peimagelayout.cpp
Outdated
@@ -591,7 +591,7 @@ PEImageLayout::EnumMemoryRegions(CLRDataEnumMemoryFlags flags) | |||
} | |||
#endif //DACCESS_COMPILE | |||
|
|||
#if defined(_WIN64) && !defined(DACCESS_COMPILE) | |||
#if defined(_TARGET_64BIT_) && !defined(DACCESS_COMPILE) | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ConvertILOnlyPE32ToPE64 and ConvertILOnlyPE32ToPE64Worker is deadcode in .NET Core. It would be better to delete it
src/vm/ceeload.cpp
Outdated
@@ -10035,9 +10035,9 @@ void Module::RestoreMethodTablePointerRaw(MethodTable ** ppMT, | |||
|
|||
if (CORCOMPILE_IS_POINTER_TAGGED(fixup)) | |||
{ | |||
#ifdef _WIN64 | |||
#ifdef _TARGET_64BIT_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not look quite right to me. I think that there is also a problem with SIZE_T
in CORCOMPILE_UNTAG_TOKEN
macro.
Maybe best to postpone this to the "interesting changes" batch.
@dotnet-bot test OSX10.12 x64 Checked Innerloop Build and Test please |
Use
_TARGET_64BIT_
instead of ambiguous_WIN64
andBIT64
Related issue: #16513