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

Fix relocs errors on riscv64 #111317

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

am11
Copy link
Member

@am11 am11 commented Jan 12, 2025

This ignores the emitter assertion in AOT mode seen here #106223 (comment). Also, missed JALR encoding/decoding in the previous PR. With these changes, the ilc is compiling all the methods and creating object file without errors.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jan 12, 2025
@am11 am11 requested a review from a team January 12, 2025 16:46
@am11 am11 mentioned this pull request Jan 12, 2025
@am11 am11 added area-NativeAOT-coreclr arch-riscv Related to the RISC-V architecture and removed area-crossgen2-coreclr labels Jan 12, 2025
Copy link
Contributor

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

@am11 am11 force-pushed the feature/nativeaot/riscv64-port branch from 2dd124f to afa2118 Compare January 12, 2025 22:16
am11 and others added 2 commits January 13, 2025 01:26
am11 and others added 2 commits January 14, 2025 02:03
Co-authored-by: Tomek Sowiński <tomeksowi@gmail.com>
@am11
Copy link
Member Author

am11 commented Jan 14, 2025

@dotnet/samsung with these patches,
-r linux-riscv64 -p:PublishAot=true -p:SysRoot=/crossrootfs/riscv64 -p:LinkerFlavor=lld

succeeds. Execution throws SIGILL.

With the default linker (LinkerFlavor=bfd), it doesn't publish. bfd throws:

/usr/bin/riscv64-linux-gnu-ld.bfd: tls_CurrentThread: TLS definition in /runtime/artifacts/bin/coreclr/linux.riscv64.Checked/aotsdk/libRuntime.WorkstationGC.a(threadstore.cpp.o) section .tbss mismatches non-TLS reference in /runtime/artifacts/bin/coreclr/linux.riscv64.Checked/aotsdk/libRuntime.WorkstationGC.a(AllocFast.S.o)

Overall it's a progress. We can take this in and continue hackathon in the tracking issue. PTAL.

src/coreclr/jit/emitriscv64.cpp Show resolved Hide resolved
@clamp03
Copy link
Member

clamp03 commented Jan 15, 2025

@am11 I removed IMAGE_REL_RISCV64_JALR in #97368 because I think its behavior of IMAGE_REL_RISCV64_JALR and IMAGE_REL_RISCV64_PC looks identical in RISCV. Do you have any reason to add IMAGE_REL_RISCV64_JALR?

@am11
Copy link
Member Author

am11 commented Jan 15, 2025

Thanks. They are handled the same way in ILC as well, only distinguishing themselves at the JIT interface level. I can remove it if the separate identity is not desired.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-riscv Related to the RISC-V architecture area-NativeAOT-coreclr 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.

5 participants