Skip to content

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

Closed

Conversation

jakobbotsch
Copy link
Member

@jakobbotsch jakobbotsch commented Mar 21, 2024

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).

This allows the LA64 and RISCV64 crossjits to build with MSVC (when
enabled in the CMakeLists.txt files for JIT and gcinfo).
@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 21, 2024
@@ -13,6 +13,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
#include "jitpch.h"
#ifdef _MSC_VER
#pragma hdrstop
#pragma warning(disable : 4244 4267)
Copy link
Member Author

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.

Copy link
Member

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:

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.

Copy link
Member Author

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!

Copy link
Member

@clamp03 clamp03 Mar 22, 2024

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. :)

Copy link
Contributor

@Bajtazar Bajtazar Mar 22, 2024

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

Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Contributor

@Bajtazar Bajtazar Mar 26, 2024

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)

@jakobbotsch
Copy link
Member Author

cc @dotnet/jit-contrib @dotnet/samsung @shushanhf

@am11
Copy link
Member

am11 commented Mar 22, 2024

when enabled in the CMakeLists.txt files for JIT and gcinfo

Do we intend to enable them in main at some point (such as part of this PR)?

@jakobbotsch
Copy link
Member Author

Do we intend to enable them in main at some point (such as part of this PR)?

No.

@MichalPetryka
Copy link
Contributor

Do we intend to enable them in main at some point (such as part of this PR)?

I've opened an issue related to this a while ago: #97712

@risc-vv
Copy link

risc-vv commented May 24, 2024

RISC-V testing failed on init-build

GIT: ae77e6d

@risc-vv
Copy link

risc-vv commented May 27, 2024

RISC-V testing failed on init-build

details

GIT: 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-vv
Copy link

risc-vv commented Jun 3, 2024

RISC-V testing failed on init-build

details

GIT: 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...

@risc-vv
Copy link

risc-vv commented Jun 28, 2024

RISC-V testing failed on init-build

GIT: ae77e6d

Full report on gist

@hez2010
Copy link
Contributor

hez2010 commented Feb 12, 2025

Superseded by #110282

@jakobbotsch jakobbotsch deleted the fix-loongarch-riscv-build-errors branch February 12, 2025 17:34
@github-actions github-actions bot locked and limited conversation to collaborators Mar 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants