Update ImportThunk for community architectures to use PC-relative addressing #121453

jkoritzinsky · 2025-11-07T19:35:31Z

They now match the ARM64 model we just moved to in #121352

Also, clean up the ARM64 import thunk code while we're at it.

They now match the ARM64 model we just moved to in dotnet#121352 Also, clean up the ARM64 emitter while we're at it.

Copilot

Pull Request Overview

This PR refactors the ImportThunk code generation for RiscV64 and LoongArch64 architectures to use a more efficient PC-relative addressing approach for loading the Module* pointer. The changes simplify the code by introducing new EmitLD(Register, ISymbolNode) overloads that emit PC-relative relocations directly in the instruction sequence, eliminating the need for manually managing offset calculations and DIR64 data slots that were required in the previous implementation.

Removes complex PC-based offset calculations and replaces them with cleaner PC-relative relocations
Introduces new EmitLD(Register, ISymbolNode) helper methods in both RiscV64Emitter and LoongArch64Emitter
Simplifies ImportThunk code by removing manual relocation management and offset tracking
Removes the _symbolOffset field and ISymbolNode.Offset override that were needed for alignment workarounds

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/Target_RiscV64/ImportThunk.cs`	Refactored to use new EmitLD helper for Module* loading, removing manual PC-relative addressing code
`src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/Target_LoongArch64/ImportThunk.cs`	Refactored to use new EmitLD helper for Module* loading, removing manual PC-relative addressing code
`src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/Target_ARM64/ImportThunk.cs`	Removed relocsOnly early return handling as part of code cleanup
`src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/ImportThunk.cs`	Removed _symbolOffset field and related offset adjustment logic no longer needed with new approach
`src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_RiscV64/RiscV64Emitter.cs`	Added new EmitLD overload for symbol-based loading; simplified EmitJMP to use the new helper
`src/coreclr/tools/Common/Compiler/DependencyAnalysis/Target_LoongArch64/LoongArch64Emitter.cs`	Added new EmitLD overload for symbol-based loading; simplified EmitJMP to use the new helper

Comments suppressed due to low confidence (1)

src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/Target_RiscV64/ImportThunk.cs:27

Dead code. The case Kind.Eager is unreachable because it's already handled by the early return at lines 18-22. This case should be removed as it can never be executed.

                case Kind.Eager:
                    break;

...t/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/Target_RiscV64/ImportThunk.cs

am11

LGTM!

cc @dotnet/samsung @LuckyXu-HF @shushanhf

am11 · 2025-11-07T19:58:51Z

Running AOT smoke tests leg (QEMU) against this PR:

linux-riscv64: https://github.com/am11/CrossRepoCITesting/actions/runs/19179619151
linux-musl-riscv64: https://github.com/am11/CrossRepoCITesting/actions/runs/19179622330
linux-loongarch64: https://github.com/am11/CrossRepoCITesting/actions/runs/19179625414
linux-musl-loongarch64: https://github.com/am11/CrossRepoCITesting/actions/runs/19179629519

…yAnalysis/ReadyToRun/Target_RiscV64/ImportThunk.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

am11 · 2025-11-07T21:14:09Z

Errors look related:

riscv64:

  Generating native image of System.Private.CoreLib for linux.riscv64.Checked. Logging to 
  /runtime/artifacts/bin/coreclr/linux.riscv64.Checked/x64/crossgen2/crossgen2 -o:/runtime/artifacts/bin/coreclr/linux.riscv64.Checked/System.Private.CoreLib.dll -r:/runtime/artifacts/bin/coreclr/linux.riscv64.Checked/IL/*.dll --targetarch:riscv64 --targetos:linux -m:/runtime/artifacts/bin/coreclr/linux.riscv64.Checked/StandardOptimizationData.mibc --embed-pgo-data -O --verify-type-and-field-layout --enable-cached-interface-dispatch-support /runtime/artifacts/bin/coreclr/linux.riscv64.Checked/IL/System.Private.CoreLib.dll --perfmap-format-version:1 --perfmap --perfmap-path:/runtime/artifacts/bin/coreclr/linux.riscv64.Checked/
  Process terminated.
  Assertion failed.
  _index != InvalidOffset
     at ILCompiler.DependencyAnalysis.EmbeddedObjectNode.get_IndexFromBeginningOfArray() in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/EmbeddedObjectNode.cs:line 38
     at ILCompiler.DependencyAnalysis.ReadyToRun.ImportThunk.EmitCode(NodeFactory factory, RiscV64Emitter& instructionEncoder, Boolean relocsOnly) in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/Target_RiscV64/ImportThunk.cs:line 36
     at ILCompiler.DependencyAnalysis.AssemblyStubNode.GetData(NodeFactory factory, Boolean relocsOnly) in /_/src/coreclr/tools/Common/Compiler/DependencyAnalysis/AssemblyStubNode.cs:line 80
     at ILCompiler.DependencyAnalysis.ObjectNode.GetStaticDependencies(NodeFactory factory) in /_/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNode.cs:line 59
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependenciesImpl(DependencyNodeCore`1 node) in /_/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 182
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ProcessMarkStack() in /_/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 257
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ComputeMarkedNodes() in /_/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 308
     at ILCompiler.ReadyToRunCodegenCompilation.Compile(String outputFile) in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs:line 393
     at ILCompiler.Program.RunSingleCompilation(Dictionary`2 inFilePaths, InstructionSetSupport instructionSetSupport, String compositeRootPath, Dictionary`2 unrootedInputFilePaths, HashSet`1 versionBubbleModulesHash, ReadyToRunCompilerContext typeSystemContext, Logger logger) in /_/src/coreclr/tools/aot/crossgen2/Program.cs:line 641
     at ILCompiler.Program.Run() in /_/src/coreclr/tools/aot/crossgen2/Program.cs:line 304
     at ILCompiler.Crossgen2RootCommand.<>c__DisplayClass211_0.<.ctor>b__0(ParseResult result) in /_/src/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs:line 268
     at System.CommandLine.Invocation.InvocationPipeline.Invoke(ParseResult parseResult)
     at ILCompiler.Program.Main(String[] args) in /_/src/coreclr/tools/aot/crossgen2/Program.cs:line 918

loongarch64:

 Generating native image of System.Private.CoreLib for linux.loongarch64.Checked. Logging to 
  /runtime/artifacts/bin/coreclr/linux.loongarch64.Checked/x64/crossgen2/crossgen2 -o:/runtime/artifacts/bin/coreclr/linux.loongarch64.Checked/System.Private.CoreLib.dll -r:/runtime/artifacts/bin/coreclr/linux.loongarch64.Checked/IL/*.dll --targetarch:loongarch64 --targetos:linux -m:/runtime/artifacts/bin/coreclr/linux.loongarch64.Checked/StandardOptimizationData.mibc --embed-pgo-data -O --verify-type-and-field-layout --enable-cached-interface-dispatch-support /runtime/artifacts/bin/coreclr/linux.loongarch64.Checked/IL/System.Private.CoreLib.dll --perfmap-format-version:1 --perfmap --perfmap-path:/runtime/artifacts/bin/coreclr/linux.loongarch64.Checked/
  Process terminated.
  Assertion failed.
  _index != InvalidOffset
     at ILCompiler.DependencyAnalysis.EmbeddedObjectNode.get_IndexFromBeginningOfArray() in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/EmbeddedObjectNode.cs:line 38
     at ILCompiler.DependencyAnalysis.ReadyToRun.ImportThunk.EmitCode(NodeFactory factory, LoongArch64Emitter& instructionEncoder, Boolean relocsOnly) in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/Target_LoongArch64/ImportThunk.cs:line 36
     at ILCompiler.DependencyAnalysis.AssemblyStubNode.GetData(NodeFactory factory, Boolean relocsOnly) in /_/src/coreclr/tools/Common/Compiler/DependencyAnalysis/AssemblyStubNode.cs:line 73
     at ILCompiler.DependencyAnalysis.ObjectNode.GetStaticDependencies(NodeFactory factory) in /_/src/coreclr/tools/Common/Compiler/DependencyAnalysis/ObjectNode.cs:line 59
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.GetStaticDependenciesImpl(DependencyNodeCore`1 node) in /_/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 182
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ProcessMarkStack() in /_/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 257
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ComputeMarkedNodes() in /_/src/coreclr/tools/aot/ILCompiler.DependencyAnalysisFramework/DependencyAnalyzer.cs:line 308
     at ILCompiler.ReadyToRunCodegenCompilation.Compile(String outputFile) in /_/src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/ReadyToRunCodegenCompilation.cs:line 393
     at ILCompiler.Program.RunSingleCompilation(Dictionary`2 inFilePaths, InstructionSetSupport instructionSetSupport, String compositeRootPath, Dictionary`2 unrootedInputFilePaths, HashSet`1 versionBubbleModulesHash, ReadyToRunCompilerContext typeSystemContext, Logger logger) in /_/src/coreclr/tools/aot/crossgen2/Program.cs:line 641
     at ILCompiler.Program.Run() in /_/src/coreclr/tools/aot/crossgen2/Program.cs:line 304
     at ILCompiler.Crossgen2RootCommand.<>c__DisplayClass211_0.<.ctor>b__0(ParseResult result) in /_/src/coreclr/tools/aot/crossgen2/Crossgen2RootCommand.cs:line 268
     at System.CommandLine.Invocation.InvocationPipeline.Invoke(ParseResult parseResult)
     at ILCompiler.Program.Main(String[] args) in /_/src/coreclr/tools/aot/crossgen2/Program.cs:line 918

jkoritzinsky · 2025-11-07T22:28:50Z

Guess I still need the relocs-only paths. I'll go fix that.

… during the relocs-only phase

Update community architectures to use PC-relative addressing

7845dd1

They now match the ARM64 model we just moved to in dotnet#121352 Also, clean up the ARM64 emitter while we're at it.

jkoritzinsky requested review from am11 and davidwrighton November 7, 2025 19:35

jkoritzinsky requested a review from MichalStrehovsky as a code owner November 7, 2025 19:35

Copilot AI review requested due to automatic review settings November 7, 2025 19:35

github-actions bot added the area-crossgen2-coreclr label Nov 7, 2025

jkoritzinsky added arch-loongarch64 arch-riscv Related to the RISC-V architecture and removed area-crossgen2-coreclr labels Nov 7, 2025

dotnet-policy-service bot assigned jkoritzinsky Nov 7, 2025

jkoritzinsky added the area-crossgen2-coreclr label Nov 7, 2025

Copilot AI reviewed Nov 7, 2025

View reviewed changes

...t/ILCompiler.ReadyToRun/Compiler/DependencyAnalysis/ReadyToRun/Target_RiscV64/ImportThunk.cs Outdated Show resolved Hide resolved

am11 approved these changes Nov 7, 2025

View reviewed changes

jkotas approved these changes Nov 7, 2025

View reviewed changes

Update src/coreclr/tools/aot/ILCompiler.ReadyToRun/Compiler/Dependenc…

60170b2

…yAnalysis/ReadyToRun/Target_RiscV64/ImportThunk.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Add back relocs-only paths to avoid reading IndexFromBeginningOfArray…

a5dde9c

… during the relocs-only phase

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update ImportThunk for community architectures to use PC-relative addressing #121453

Update ImportThunk for community architectures to use PC-relative addressing #121453

jkoritzinsky commented Nov 7, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

am11 left a comment

Uh oh!

am11 commented Nov 7, 2025

Uh oh!

am11 commented Nov 7, 2025

Uh oh!

jkoritzinsky commented Nov 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Update ImportThunk for community architectures to use PC-relative addressing #121453

Are you sure you want to change the base?

Update ImportThunk for community architectures to use PC-relative addressing #121453

Conversation

jkoritzinsky commented Nov 7, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

am11 left a comment

Choose a reason for hiding this comment

Uh oh!

am11 commented Nov 7, 2025

Uh oh!

am11 commented Nov 7, 2025

Uh oh!

jkoritzinsky commented Nov 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants