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

build: enable Clang-cl Windows builds #35433

Closed
wants to merge 12 commits into from
Prev Previous commit
Next Next commit
hack ICU
  • Loading branch information
targos committed Apr 30, 2024
commit 711a822f831f8c46f5935280257d3b7c8769a56a
3 changes: 2 additions & 1 deletion deps/icu-small/source/tools/toolutil/pkg_genc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -846,8 +846,9 @@ getArchitecture(uint16_t *pCPU, uint16_t *pBits, UBool *pIsBigEndian, const char
// for all architectures though.
# if defined(_M_IX86)
*pCPU = IMAGE_FILE_MACHINE_I386;
// TODO: detect ARM64
# else
*pCPU = IMAGE_FILE_MACHINE_UNKNOWN;
*pCPU = IMAGE_FILE_MACHINE_AMD64;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is for #34201

Comment on lines +849 to +851
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried an ARM64 build and got an error connected to these changes.

After some investigation, I found that dealing with a TODO: detect ARM64 added here would be tricky. This file is a part of a host project, thus it is always compiled for x64 in our case. Ideally, we'd use _M_ARM64/_M_AMD64 and set different IMAGE_FILE_MACHINE_... based on that in a target architecture project, but that cannot be used here.

What we can do, is add another option to genccode_host (genccode.c) that we'd use for setting the *pCPU. The issue with it is that the files I'd change in the process are part of the deps so we'd be making a floating patch on similar to the ones we have on V8, which is not great, but currently I see no other way (although there might be some).

# endif
# if defined(_M_IA64) || defined(_M_AMD64) || defined (_M_ARM64)
*pBits = 64; // Doesn't seem to be used for anything interesting though?
Expand Down