Skip to content

Remove obsolete and redundant VM assembly - #133300

Closed
EgorBo wants to merge 3 commits into
dotnet:mainfrom
EgorBo:cleanup-vm-assembly
Closed

Remove obsolete and redundant VM assembly#133300
EgorBo wants to merge 3 commits into
dotnet:mainfrom
EgorBo:cleanup-vm-assembly

Conversation

@EgorBo

@EgorBo EgorBo commented Sep 5, 2026

Copy link
Copy Markdown
Member

Remove unused VM assembly helpers and their leftover declarations, imports, and state.

Generated by GitHub Copilot.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b5e1456-5743-4714-ba72-d519f1552a1d
Copilot AI lite review requested due to automatic review settings September 5, 2026 10:21
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@EgorBo
EgorBo marked this pull request as draft September 5, 2026 10:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

It modifies low-level, platform-specific CoreCLR assembly and write-barrier patching, which has high regression risk across rarely-built configurations.

Pull request overview

This PR removes obsolete CoreCLR VM assembly and associated support code across multiple architectures, while simplifying the AMD64 write-barrier placeholder by reserving an explicit fixed-size buffer used by WriteBarrierManager patching.

Changes:

  • Replaces the duplicated AMD64 JIT_WriteBarrier placeholder implementation with a fixed 256-byte buffer (new JIT_WRITE_BARRIER_BUFFER_SIZE constant).
  • Removes unused write-barrier location globals, unused x86/AMD64 helper routines, and stale comments tied to deleted stubs.
  • Deletes dead/disabled fixup and stack-probing stubs (e.g., FEATURE_PREJIT LoongArch64 stubs, ARM stack-probe emitter chain), plus unused imports.
File summaries
File Description
src/coreclr/vm/virtualcallstub.cpp Removes obsolete debugger-only extern tied to deleted patch label.
src/coreclr/vm/threads.cpp Cleans up write-barrier location globals and initialization paths (notably x86 vs non-x86).
src/coreclr/vm/loongarch64/asmhelpers.S Deletes FEATURE_PREJIT NGEN-era fixup stubs and patch labels.
src/coreclr/vm/i386/jithelp.S Removes unused x86 write-barrier location global storage.
src/coreclr/vm/i386/jithelp.asm Removes unused x86 write-barrier location global storage (MASM).
src/coreclr/vm/i386/cgenx86.cpp Updates comment to reflect remaining live worker paths.
src/coreclr/vm/i386/asmhelpers.S Removes Unix-side x86 FPU helpers now treated as Windows-only.
src/coreclr/vm/frames.cpp Updates debugger trace comment to match remaining behavior.
src/coreclr/vm/excep.cpp Makes the x86 FPU-state holder clearly Windows-only by moving #ifndef TARGET_UNIX.
src/coreclr/vm/cgensys.h Removes obsolete VirtualMethodFixup* declarations.
src/coreclr/vm/arm64/asmhelpers.asm Removes stale imports and deletes an unused COM return helper.
src/coreclr/vm/arm/cgencpu.h Removes unused ARM stack-probing APIs and related emitter helpers.
src/coreclr/vm/arm/asmhelpers.S Removes permanently-disabled redirection and stack-probe stub code.
src/coreclr/vm/amd64/patchedcode.S Replaces placeholder barrier body with fixed-size buffer fill.
src/coreclr/vm/amd64/patchedcode.asm Same fixed-size buffer change for MASM build.
src/coreclr/vm/amd64/getstate.asm Deletes unused AMD64 helpers (GetRBP, VM FPFillR8, get_cycle_count).
src/coreclr/vm/amd64/asmconstants.h Introduces JIT_WRITE_BARRIER_BUFFER_SIZE constant (0x100).
Review details
  • Files reviewed: 17/17 changed files
  • Comments generated: 0
  • Review effort level: Lite

EgorBo and others added 2 commits September 5, 2026 12:29
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b5e1456-5743-4714-ba72-d519f1552a1d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4b5e1456-5743-4714-ba72-d519f1552a1d

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-architecture assembly removals require human validation, and one stale cross-file comment remains.

Review details

Suppressed comments (1)

src/coreclr/vm/amd64/getstate.asm:25

  • Removing FPFillR8 here leaves src/coreclr/debug/di/amd64/FloatConversion.asm:4-8 claiming that the duplicate implementation still exists in this file and must remain in sync. Please remove or update those comments as part of this cleanup so they do not point maintainers at nonexistent code.
        end
  • Files reviewed: 14/14 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@EgorBo
EgorBo marked this pull request as ready for review September 5, 2026 11:07
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@EgorBo EgorBo closed this Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants