ICU build issue with clang-cl

[targos]

I'm experimenting with Visual Studio's Clang support and have an issue with ICU:

There's only one line with an error while building tools\icu\icudata.vcxproj

  ..\..\out\Release\\obj\global_intermediate\icudt67l_dat.obj: unknown machine: 0

/cc @srl295

Diff to get there

```diff diff --git a/deps/zlib/adler32_simd.c b/deps/zlib/adler32_simd.c index f8b07297b93..d5b63727a48 100644 --- a/deps/zlib/adler32_simd.c +++ b/deps/zlib/adler32_simd.c @@ -50,7 +50,7 @@ #define NMAX 5552 #if defined(ADLER32_SIMD_SSSE3) -#ifndef __GNUC__ +#if !defined(__GNUC__) && !defined(__clang__) #define __attribute__() #endif diff --git a/deps/zlib/crc32_simd.c b/deps/zlib/crc32_simd.c index 27481847e97..4c5ab42701a 100644 --- a/deps/zlib/crc32_simd.c +++ b/deps/zlib/crc32_simd.c @@ -8,7 +8,7 @@ #include "crc32_simd.h" #if defined(CRC32_SIMD_SSE42_PCLMUL) -#ifndef __GNUC__ +#if !defined(__GNUC__) && !defined(__clang__) #define __attribute__() #endif diff --git a/deps/zlib/crc_folding.c b/deps/zlib/crc_folding.c index 54f4b5c9401..1e4ec5de64c 100644 --- a/deps/zlib/crc_folding.c +++ b/deps/zlib/crc_folding.c @@ -21,9 +21,11 @@ #include #include #include +#include +#include #include -#ifndef __GNUC__ +#if !defined(__GNUC__) && !defined(__clang__) #define __attribute__() #endif diff --git a/tools/gyp/pylib/gyp/generator/msvs.py b/tools/gyp/pylib/gyp/generator/msvs.py index 09abadb1bcb..1c960aefca3 100644 --- a/tools/gyp/pylib/gyp/generator/msvs.py +++ b/tools/gyp/pylib/gyp/generator/msvs.py @@ -2799,7 +2799,7 @@ def _GetMSBuildLocalProperties(msbuild_toolset): if msbuild_toolset: properties = [ ['PropertyGroup', {'Label': 'Locals'}, - ['PlatformToolset', msbuild_toolset], + ['PlatformToolset', 'ClangCL'], ] ] return properties ```

[gengjiawen]

cc @nodejs/i18n

[srl295]

@targos have not heard of this, may be good to file an ICU bug upstream at https://unicode-org.atlassian.net // @jefgen

[srl295]

may have to fall back to .c file builds for data if the assembly generayion is getting confused

[gengjiawen]

I am not familiar with icu, which part involved into this ? Is unknown machine: 0 comes from icu code or clang ?

[targos]

I think the problem comes from this file: https://github.com/nodejs/node/blob/a51436cbea0d216f260453321a15fdce72ee28d3/deps/icu-small/source/tools/toolutil/pkg_genc.cpp

During compilation, the following printf: https://github.com/nodejs/node/blob/a51436cbea0d216f260453321a15fdce72ee28d3/deps/icu-small/source/tools/toolutil/pkg_genc.cpp#L1156

  genccode: using architecture cpu=0 bits=64 big-endian=0

[jefgen]

The value of 0 for cpu comes from here: https://github.com/nodejs/node/blob/a51436cbea0d216f260453321a15fdce72ee28d3/deps/icu-small/source/tools/toolutil/pkg_genc.cpp#L787

From the comment in the file:

// link.exe will link an IMAGE_FILE_MACHINE_UNKNOWN data-only .obj file
// no matter what architecture it is targeting (though other values are
// required to match). Unfortunately, the variable name decoration/mangling
// is slightly different on x86, which means we can't use the UNKNOWN type
// for all architectures though.

I wonder if maybe the linker for clang doesn't handle IMAGE_FILE_MACHINE_UNKNOWN the same as the linker for MSVC...?

IIRC, the reason IMAGE_FILE_MACHINE_UNKNOWN was set, was to produce a generic .obj file that could be used for both x64 or ARM64, in order to support cross-compiling for ARM64 on x64.

If you don't need to cross-compile ARM64 (on a x64 host) then perhaps this could be changed to set an explicit CPU arch when building with clang.

Perhaps something like:

#elif U_PLATFORM_HAS_WIN32_API
        // Windows always runs in little-endian mode.
        *pIsBigEndian = FALSE;

        // Note: The various _M_<arch> macros are predefined by the MSVC compiler based
        // on the target compilation architecture.
        // https://docs.microsoft.com/cpp/preprocessor/predefined-macros

        // link.exe will link an IMAGE_FILE_MACHINE_UNKNOWN data-only .obj file
        // no matter what architecture it is targeting (though other values are
        // required to match). Unfortunately, the variable name decoration/mangling
        // is slightly different on x86, which means we can't use the UNKNOWN type
        // for all architectures though.
#   if defined(_M_IX86)
        *pCPU = IMAGE_FILE_MACHINE_I386;
#   else
#ifndef (__clang__)
        // the linker for clang-cl doesn't work with unknown.
        *pCPU = IMAGE_FILE_MACHINE_UNKNOWN;
#elif defined(_M_AMD64)
        *pCPU = IMAGE_FILE_MACHINE_AMD64;
#endif /* ifndef (__clang__) */
#   endif

[targos]

If you don't need to cross-compile ARM64 (on a x64 host)

Unfortunately, we do cross-compile ARM64 on x64 hosts.