-
Notifications
You must be signed in to change notification settings - Fork 5k
JIT: Fix LA64 and RISCV64 build errors on Windows #100099
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
JIT: Fix LA64 and RISCV64 build errors on Windows #100099
Conversation
This allows the LA64 and RISCV64 crossjits to build with MSVC (when enabled in the CMakeLists.txt files for JIT and gcinfo).
@@ -13,6 +13,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX | |||
#include "jitpch.h" | |||
#ifdef _MSC_VER | |||
#pragma hdrstop | |||
#pragma warning(disable : 4244 4267) |
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.
There are a number of "possible loss of data" warnings related to the helpers like NBitMask
that required adding a bunch of casts in many places in this file. I went with just disabling these warnings since I did not want to uglify all of that code. Perhaps there is a prettier way to fix these warnings.
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.
Disabling this warning is SDL violation:
runtime/eng/native/configurecompiler.cmake
Line 847 in ac07ea6
add_compile_options($<$<COMPILE_LANGUAGE:C,CXX>:/we4244>) # 'conversion' conversion from 'type1' to 'type2', possible loss of data |
It is only ok as long as this code is not getting compiled in official builds.
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.
@dotnet/samsung Can you please provide a patch that fixes the warnings in your preferred way without resorting to disabling the warning? Thanks!
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.
@jakobbotsch Thank you.
@Bajtazar Could you fix the warnings in emitriscv64.cpp
? The most of warnings are casts related to helpers which you have refactored. I think you can fix in prettier way. :)
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.
Hi, I prepared a patch that fixes warnings in the emitriscv64.cpp
file
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.
@Bajtazar I think the patch can be merged separately. Could you make another PR for your patch?
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.
Thank you!
@Bajtazar I think the patch can be merged separately. Could you make another PR for your patch?
You can open a separate PR or I can include it in this change. Up to you.
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.
@clamp03 @jakobbotsch I've opened a separate PR with fixes (#100270)
cc @dotnet/jit-contrib @dotnet/samsung @shushanhf |
Do we intend to enable them in |
No. |
I've opened an issue related to this a while ago: #97712 |
RISC-V testing failed on init-buildGIT: ae77e6d |
RISC-V testing failed on init-builddetailsGIT: ae77e6d Cloning into 'runtime'...
Updating files: 19% (11213/58591)
Updating files: 20% (11719/58591)
Updating files: 21% (12305/58591)
Updating files: 22% (12891/58591)
Updating files: 23% (13476/58591)
Updating files: 24% (14062/58591)
Updating files: 25% (14648/58591)
Updating files: 26% (15234/58591)
Updating files: 27% (15820/58591)
Updating files: 28% (16406/58591)
Updating files: 29% (16992/58591)
Updating files: 30% (17578/58591)
Updating files: 31% (18164/58591)
Updating files: 32% (18750/58591)
Updating files: 33% (19336/58591)
Updating files: 34% (19921/58591)
Updating files: 35% (20507/58591)
Updating files: 36% (21093/58591)
Updating files: 37% (21679/58591)
Updating files: 38% (22265/58591)
Updating files: 39% (22851/58591)
Updating files: 40% (23437/58591)
Updating files: 41% (24023/58591)
Updating files: 41% (24321/58591)
Updating files: 42% (24609/58591)
Updating files: 43% (25195/58591)
Updating files: 44% (25781/58591)
Updating files: 45% (26366/58591)
Updating files: 46% (26952/58591)
Updating files: 47% (27538/58591)
Updating files: 48% (28124/58591)
Updating files: 49% (28710/58591)
Updating files: 50% (29296/58591)
Updating files: 51% (29882/58591)
Updating files: 52% (30468/58591)
Updating files: 53% (31054/58591)
Updating files: 54% (31640/58591)
Updating files: 55% (32226/58591)
Updating files: 56% (32811/58591)
Updating files: 57% (33397/58591)
Updating files: 58% (33983/58591)
Updating files: 59% (34569/58591)
Updating files: 60% (35155/58591)
Updating files: 61% (35741/58591)
Updating files: 62% (36327/58591)
Updating files: 63% (36913/58591)
Updating files: 64% (37499/58591)
Updating files: 65% (38085/58591)
Updating files: 65% (38309/58591)
Updating files: 66% (38671/58591)
Updating files: 67% (39256/58591)
Updating files: 68% (39842/58591)
Updating files: 69% (40428/58591)
Updating files: 70% (41014/58591)
Updating files: 71% (41600/58591)
Updating files: 72% (42186/58591)
Updating files: 73% (42772/58591)
Updating files: 74% (43358/58591)
Updating files: 75% (43944/58591)
Updating files: 76% (44530/58591)
Updating files: 77% (45116/58591)
Updating files: 78% (45701/58591)
Updating files: 79% (46287/58591)
Updating files: 80% (46873/58591)
Updating files: 81% (47459/58591)
Updating files: 82% (48045/58591)
Updating files: 83% (48631/58591)
Updating files: 84% (49217/58591)
Updating files: 85% (49803/58591)
Updating files: 86% (50389/58591)
Updating files: 87% (50975/58591)
Updating files: 87% (51514/58591)
Updating files: 88% (51561/58591)
Updating files: 89% (52146/58591)
Updating files: 90% (52732/58591)
Updating files: 91% (53318/58591)
Updating files: 92% (53904/58591)
Updating files: 93% (54490/58591)
Updating files: 94% (55076/58591)
Updating files: 95% (55662/58591)
Updating files: 96% (56248/58591)
Updating files: 97% (56834/58591)
Updating files: 98% (57420/58591)
Updating files: 99% (58006/58591)
Updating files: 100% (58591/58591)
Updating files: 100% (58591/58591), done.
Switched to a new branch 'build-net'
Checking patch src/coreclr/inc/corinfo.h...
Hunk #1 succeeded at 3405 (offset -21 lines).
Checking patch src/coreclr/inc/gcinfotypes.h...
Checking patch src/coreclr/inc/palclr.h...
Hunk #1 succeeded at 612 (offset 2 lines).
Checking patch src/coreclr/jit/codegenloongarch64.cpp...
Hunk #1 succeeded at 2793 (offset -137 lines).
error: while searching for:
if (regArgNum > 0)
{
instruction ins;
for (int i = MAX_REG_ARG - 1; i >= 0; i--)
{
if (regArg[i] > 0)
error: patch failed: src/coreclr/jit/codegenloongarch64.cpp:8346
error: src/coreclr/jit/codegenloongarch64.cpp: patch does not apply
Checking patch src/coreclr/jit/codegenriscv64.cpp...
error: while searching for:
noway_assert(isFramePointerUsed()); // localloc requires Frame Pointer to be established since SP changes
noway_assert(genStackLevel == 0); // Can't have anything on the stack
const target_ssize_t pageSize = compiler->eeGetPageSize();
// According to RISC-V Privileged ISA page size is 4KiB
noway_assert(pageSize == 0x1000);
error: patch failed: src/coreclr/jit/codegenriscv64.cpp:2013
error: src/coreclr/jit/codegenriscv64.cpp: patch does not apply
Checking patch src/coreclr/jit/emitriscv64.cpp...
Hunk #2 succeeded at 4435 (offset 36 lines).
Checking patch src/coreclr/pal/inc/rt/ntimage.h...
|
RISC-V testing failed on init-builddetailsGIT: ae77e6d Cloning into 'runtime'...
Updating files: 18% (10978/58653)
Updating files: 19% (11145/58653)
Updating files: 20% (11731/58653)
Updating files: 21% (12318/58653)
Updating files: 22% (12904/58653)
Updating files: 23% (13491/58653)
Updating files: 24% (14077/58653)
Updating files: 25% (14664/58653)
Updating files: 26% (15250/58653)
Updating files: 27% (15837/58653)
Updating files: 28% (16423/58653)
Updating files: 29% (17010/58653)
Updating files: 30% (17596/58653)
Updating files: 31% (18183/58653)
Updating files: 32% (18769/58653)
Updating files: 33% (19356/58653)
Updating files: 34% (19943/58653)
Updating files: 35% (20529/58653)
Updating files: 36% (21116/58653)
Updating files: 37% (21702/58653)
Updating files: 38% (22289/58653)
Updating files: 39% (22875/58653)
Updating files: 40% (23462/58653)
Updating files: 40% (23712/58653)
Updating files: 41% (24048/58653)
Updating files: 42% (24635/58653)
Updating files: 43% (25221/58653)
Updating files: 44% (25808/58653)
Updating files: 45% (26394/58653)
Updating files: 46% (26981/58653)
Updating files: 47% (27567/58653)
Updating files: 48% (28154/58653)
Updating files: 49% (28740/58653)
Updating files: 50% (29327/58653)
Updating files: 51% (29914/58653)
Updating files: 52% (30500/58653)
Updating files: 53% (31087/58653)
Updating files: 54% (31673/58653)
Updating files: 55% (32260/58653)
Updating files: 56% (32846/58653)
Updating files: 57% (33433/58653)
Updating files: 58% (34019/58653)
Updating files: 59% (34606/58653)
Updating files: 60% (35192/58653)
Updating files: 61% (35779/58653)
Updating files: 62% (36365/58653)
Updating files: 63% (36952/58653)
Updating files: 63% (37304/58653)
Updating files: 64% (37538/58653)
Updating files: 65% (38125/58653)
Updating files: 66% (38711/58653)
Updating files: 67% (39298/58653)
Updating files: 68% (39885/58653)
Updating files: 69% (40471/58653)
Updating files: 70% (41058/58653)
Updating files: 71% (41644/58653)
Updating files: 72% (42231/58653)
Updating files: 73% (42817/58653)
Updating files: 74% (43404/58653)
Updating files: 75% (43990/58653)
Updating files: 76% (44577/58653)
Updating files: 77% (45163/58653)
Updating files: 78% (45750/58653)
Updating files: 79% (46336/58653)
Updating files: 80% (46923/58653)
Updating files: 81% (47509/58653)
Updating files: 82% (48096/58653)
Updating files: 83% (48682/58653)
Updating files: 84% (49269/58653)
Updating files: 85% (49856/58653)
Updating files: 86% (50442/58653)
Updating files: 87% (51029/58653)
Updating files: 87% (51429/58653)
Updating files: 88% (51615/58653)
Updating files: 89% (52202/58653)
Updating files: 90% (52788/58653)
Updating files: 91% (53375/58653)
Updating files: 92% (53961/58653)
Updating files: 93% (54548/58653)
Updating files: 94% (55134/58653)
Updating files: 95% (55721/58653)
Updating files: 96% (56307/58653)
Updating files: 97% (56894/58653)
Updating files: 98% (57480/58653)
Updating files: 99% (58067/58653)
Updating files: 100% (58653/58653)
Updating files: 100% (58653/58653), done.
Switched to a new branch 'build-net'
Checking patch src/coreclr/inc/corinfo.h...
Hunk #1 succeeded at 3405 (offset -21 lines).
Checking patch src/coreclr/inc/gcinfotypes.h...
Checking patch src/coreclr/inc/palclr.h...
Hunk #1 succeeded at 612 (offset 2 lines).
Checking patch src/coreclr/jit/codegenloongarch64.cpp...
Hunk #1 succeeded at 2793 (offset -137 lines).
error: while searching for:
if (regArgNum > 0)
{
instruction ins;
for (int i = MAX_REG_ARG - 1; i >= 0; i--)
{
if (regArg[i] > 0)
error: patch failed: src/coreclr/jit/codegenloongarch64.cpp:8346
error: src/coreclr/jit/codegenloongarch64.cpp: patch does not apply
Checking patch src/coreclr/jit/codegenriscv64.cpp...
error: while searching for:
noway_assert(isFramePointerUsed()); // localloc requires Frame Pointer to be established since SP changes
noway_assert(genStackLevel == 0); // Can't have anything on the stack
const target_ssize_t pageSize = compiler->eeGetPageSize();
// According to RISC-V Privileged ISA page size is 4KiB
noway_assert(pageSize == 0x1000);
error: patch failed: src/coreclr/jit/codegenriscv64.cpp:2013
error: src/coreclr/jit/codegenriscv64.cpp: patch does not apply
Checking patch src/coreclr/jit/emitriscv64.cpp...
Hunk #2 succeeded at 4506 (offset 107 lines).
Checking patch src/coreclr/pal/inc/rt/ntimage.h...
|
Superseded by #110282 |
This allows the LA64 and RISCV64 crossjits to build with MSVC (when enabled in the CMakeLists.txt files for JIT and gcinfo).
The motivation is to make it easier to validate that we don't break RISC-V/LA64 builds during broad refactorings (case in point -- #100091).