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

[RISC-V] Fix build error #85232

Closed
wants to merge 1 commit into from
Closed

[RISC-V] Fix build error #85232

wants to merge 1 commit into from

Conversation

clamp03
Copy link
Member

@clamp03 clamp03 commented Apr 24, 2023

- Fixed build error due to no return value in non-void function
  (ins_Move_Extend)
- dotnet#84834
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 24, 2023
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Apr 24, 2023
@ghost
Copy link

ghost commented Apr 24, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details
Author: clamp03
Assignees: -
Labels:

area-CodeGen-coreclr, community-contribution

Milestone: -

@clamp03 clamp03 changed the title [RISCV64] Fix build error [RISC-V] Fix build error Apr 24, 2023
@shushanhf
Copy link
Contributor

shushanhf commented Apr 24, 2023

This PR is after LoongArch64's #85216 and conflict.
It should be pushed later.

This is just a duplication of #85216, so I had merged it to #85216.

By the way, if you just a duplication of LoongArch64, please don't push RISCV before LoongArch64 merged to avoid conflicting.

@clamp03
Copy link
Member Author

clamp03 commented Apr 24, 2023

This PR is after LoongArch64's #85216 and conflict. It should be pushed later.

This is just a duplication of #85216, so I had merged it to #85216.

By the way, if you just a duplication of LoongArch64, please don't push RISCV before LoongArch64 merged to avoid conflicting.

@shushanhf Thank you. When I saw this build error, before fixing this error, I searched and found your patch which fix only LA64. So I pushed this for RISC64. If it conflicts, then I would fix conflicts. Actually, someone's patches can be conflict with mine. I don't care and just fix it. I cannot prevent all PRs from others.

Do you have any plan to add CI for LA64? I think LA64 build and tests frequently fails. (RISC-V too. 😢 ) It already occurs 8 times #85216, #85151, #85140, #84971, #84970, #84970, #84960 and #83370 for only March and April. Unfortunately, we did 2 times. I am very sorry about that. However, we cannot prevent all mistakes even if we really carefully check. I think CI is very good option to keep LA64 safely. If you don't mind, I can help. Could you please let me know how to build LA64?
For RISC-V, patches are ready for build CI with helps of community members (thank you @am11). dotnet/dotnet-buildtools-prereqs-docker#854 and clamp03@8b2d357

@clamp03 clamp03 closed this Apr 24, 2023
@shushanhf
Copy link
Contributor

Do you have any plan to add CI for LA64? I think LA64 build and tests frequently fails. (RISC-V too. )

We have constructed a local CI for LA64.
I will add LA64's CI to community later. Because I am busying with amending the LA64‘s code.

Thanks

@clamp03
Copy link
Member Author

clamp03 commented Apr 24, 2023

@shushanhf One more notification, I fixed an error in RISC-V. bed8b9c. When optimizing branch is applied, the patch reverses condition in GT_JCMP. In my opinion, it is related to LA64 too. If you think it needs for LA64 ASAP, you can copy codes for LA64 and make another PR. I will resolve a conflict in my side. I think it takes long time to merge my PR. :)

+ From next time, if you don't mind, I hope requesting you to add RISCV if your patch is useful for both LA64 and RISCV. And I will add you as a CC if my PR is good for both LA64 and RISCV. I think we have many things we can share and help.
Thank you.

@clamp03 clamp03 deleted the fixbuilderror branch April 24, 2023 02:43
@shushanhf
Copy link
Contributor

@shushanhf One more notification, I fixed an error in RISC-V. bed8b9c. When optimizing branch is applied, the patch reverses condition in GT_JCMP. In my opinion,

We had already fixed this for LA64.

@ghost ghost locked as resolved and limited conversation to collaborators May 24, 2023
@clamp03 clamp03 self-assigned this Sep 7, 2023
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 community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants