Rename cross compilation related defines#2256
Conversation
src/coreclr/clr.featuredefines.props
Outdated
There was a problem hiding this comment.
I would be nice to delete FeaturePal as well (in future PR).
5fbd3db to
6095894
Compare
We have choose |
It was currently set based on the host architecture. It looks wrong in most places, but I wanted to make this mostly a non-functional change. Given the JIT/crossgen supports cross bitness builds, I didn't want to mess with this here. I also switched the PLATFORM_UNIX to map to HOST_UNIX on the similar rationale. |
|
Ok, hope most them are going to be flipped to be |
|
I certainly want to minimize |
|
I had originally been doing the bulk edit replaces in my editor. Turns out my editor was skipping some files for some reason.... Anyways I decided it was less error prone to script this. So I wrote a bash/git/sed script to replace a single editor within a git subtree. Then I wrote a script for all the bulk edits. I kept the commits more separate to ease review burden. The bulk search replace are separate commits. The manual edits are separate commits.
|
b61329b to
75157b4
Compare
This patch was a machine generated search and replace of
identifiers. The replacement was done in the order listed
for the path specified.
A regex was used to find the identifiers.
s/(^|[^A-CE-Za-z0-9_])${find}($|[^A-Za-z0-9_])
The 'D' character was specifically allowed a s a prefix
to catch add_definitions(-D${find}).
The patch also reverts all managed code changes after replacement.
_ARM_ -> HOST_ARM in src/coreclr
_ARM64_ -> HOST_ARM64 in src/coreclr
_AMD64_ -> HOST_AMD64 in src/coreclr
_X86_ -> HOST_X86 in src/coreclr
_HOST_UNIX_ -> HOST_UNIX in src/coreclr
_HOST_AMD64_ -> HOST_AMD64 in src/coreclr
_HOST_ARM64_ -> HOST_ARM64 in src/coreclr
_HOST_ARM_ -> HOST_ARM in src/coreclr
_HOST_X86_ -> HOST_X86 in src/coreclr
DBG_TARGET_AMD64 -> TARGET_AMD64 in src/coreclr
DBG_TARGET_ARM64 -> TARGET_ARM64 in src/coreclr
DBG_TARGET_ARM -> TARGET_ARM in src/coreclr
DBG_TARGET_X86 -> TARGET_X86 in src/coreclr
_TARGET_AMD64_ -> TARGET_AMD64 in src/coreclr
_TARGET_ARM_ -> TARGET_ARM in src/coreclr
_TARGET_ARM64_ -> TARGET_ARM64 in src/coreclr
_TARGET_X86_ -> TARGET_X86 in src/coreclr
_TARGET_ARMARCH_ -> TARGET_ARMARCH in src/coreclr
_TARGET_XARCH_ -> TARGET_XARCH in src/coreclr
_HOST_64BIT_ -> HOST_64BIT in src/coreclr
BIT64 -> HOST_64BIT in src/coreclr
_TARGET_64BIT_ -> TARGET_64BIT in src/coreclr
DBG_TARGET_64BIT -> TARGET_64BIT in src/coreclr
HOST_IS_WINDOWS_OS -> HOST_WINDOWS in src/coreclr
PLATFORM_UNIX -> HOST_UNIX in src/coreclr/*.cmake
PLATFORM_WINDOWS -> HOST_WINDOWS in src/coreclr/*.cmake
_TARGET_MAC64 -> TARGET_OSX in src/coreclr
FEATURE_PAL -> TARGET_UNIX in src/coreclr
_TARGET_UNIX_ -> TARGET_UNIX in src/coreclr
_TARGET_WINDOWS_ -> TARGET_WINDOWS in src/coreclr
PLATFORM_UNIX -> TARGET_UNIX in src/coreclr
PLATFORM_WINDOWS -> TARGET_WINDOWS in src/coreclr
4735c94 to
08e3951
Compare
Remove unused defines Remove BIT32 Remove DBG_TARGET_AMD64_UNIX Remove DBG_TARGET_ARM64_UNIX Remove DBG_TARGET_ARM_UNIX Remove DBG_TARGET_32BIT Fixes for HOST_<arch> rename Move TARGET_<Arch> and TARGET_<bit> Move from clrdefinitions.cmake to configurecompiler.cmake so it is used globally. More jit.h
|
fwiw, this change deserved to be (and still deserves to be) much more widely before being merged. Anyone with in-progress changes that added architecture-specific code (for example), then rebased, might not get the behavior they expect because previously-working @dotnet/jit-contrib fyi: many global Also, can we change |
There is discussion about it here: #31659 (comment) . We have OSX name in the number of places today that we cannot change (e.g. https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.osplatform.osx?view=netframework-4.8). If we want to change it to something, I would vote for DARWIN. DARWIN should be immune to marketing whims. |
|
Can we make the old names cause errors for some interim period, so it's more obvious they won't work anymore? Otherwise stuff may just silently get dropped. |
|
I filed #31732 to clean up some inconsistencies in the JIT |
|
@dotnet/jit-contrib My apologies for not socializing this earlier. I had been concerned that my PR would shear with respect to master. That somehow I would have missed a rename.... I checked immediately after I merged so that I could prevent an outage. I didn't really anticipate the possibility that other PRs might go in after w/o having merge conflicts with this one.... @AndyAyersMS I am not sure how to make use of an undefined macro cause a build break. Something like #define _TARGET_ARM_ static_assert(BadDefine != BadDefine, "_TARGET_ARM_ macro is obsolete. Use TARGET_ARM")Might work in some circumstances, but not in all. I don't think it would work in the most common cases. Particularly: #ifdef _TARGET_ARM_#if defined(_TARGET_ARM_) |
|
Will |
|
For non-MSVC compilers, that #define WARN(msg) WARN2(GCC warning msg)
#define WARN2(s) _Pragma(#s)
#define DEPRECATE_MACRO WARN("Macro is deprecated")And then use it like so: #define _TARGET_ARM_ DEPRECATE_MACROI don't know, however, if there's something similar for C++ or even MSVC, though. |
|
While this mechanism works with clang, it doesn't help our purpose. The problem is that it only warns for |
A future PR will rename some of the TARGET_* to HOST_* and vice-versa as needed. This mostly a brute force search and replace.