Skip to content

Revert "[RyuJit/WASM] Remove the special prolog handling" - #130148

Closed
jkotas wants to merge 2 commits into
mainfrom
revert-128721-Emit-PrologIG-Exec
Closed

Revert "[RyuJit/WASM] Remove the special prolog handling"#130148
jkotas wants to merge 2 commits into
mainfrom
revert-128721-Emit-PrologIG-Exec

Conversation

@jkotas

@jkotas jkotas commented Jul 2, 2026

Copy link
Copy Markdown
Member

Reverts #128721

Fixes #130145

Copilot AI review requested due to automatic review settings July 2, 2026 23:07
@github-actions github-actions Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 2, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

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.

Pull request overview

This PR reverts #128721 by reintroducing explicit “currently generating prolog/epilog” state in Compiler and using that state throughout codegen/unwind/emitter logic, including moving WASM register-parameter homing out of the prolog.

Changes:

  • Introduce Compiler::compGeneratingProlog / compGeneratingEpilog and scope them via ScopedSetVariable in prolog/epilog (and funclet prolog/epilog) generation paths.
  • Remove emitter::emitGeneratingPrologOrFuncletProlog() / emitGeneratingEpilogOrFuncletEpilog() and convert most call sites to the new compiler flags.
  • For WASM, split genHomeRegisterParams and inject the real work into the first basic block (genHomeRegisterParamsOutsideProlog) to avoid prolog size restrictions.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/coreclr/jit/unwindx86.cpp Switch prolog/epilog assertions to compiler generation flags.
src/coreclr/jit/unwindwasm.cpp Switch prolog/epilog assertions to compiler generation flags.
src/coreclr/jit/unwindriscv64.cpp Switch prolog checks in CFI/unwind helpers to compiler generation flags.
src/coreclr/jit/unwindloongarch64.cpp Switch prolog checks in CFI/unwind helpers to compiler generation flags.
src/coreclr/jit/unwindarmarch.cpp Switch prolog/epilog checks in unwind helpers to compiler generation flags.
src/coreclr/jit/unwindarm64.cpp Switch prolog checks (incl. PAC CFI emission gating) to compiler generation flags.
src/coreclr/jit/unwindamd64.cpp Switch prolog/epilog checks in unwind helpers to compiler generation flags.
src/coreclr/jit/unwind.cpp Switch CFI helper assertions/conditions to compiler generation flags.
src/coreclr/jit/scopeinfo.cpp Switch prolog scope assertions to compiler generation flags.
src/coreclr/jit/regset.cpp Use compiler generation flags to allow register-modification tracking during prolog/epilog.
src/coreclr/jit/emitxarch.cpp Switch prolog-only assertion to compiler generation flags.
src/coreclr/jit/emitriscv64.cpp Gate RISC-V compressed/immediate emission decisions on compiler generation flags.
src/coreclr/jit/emitpub.h Remove emitGenerating* APIs from the emitter public surface.
src/coreclr/jit/emit.h Adjust emitter initialization logic related to current-IG state.
src/coreclr/jit/emit.cpp Remove emitGenerating* helpers; switch key asserts to compiler generation flags; minor IG init adjustment.
src/coreclr/jit/compiler.h Add compGeneratingProlog / compGeneratingEpilog flags to Compiler.
src/coreclr/jit/codegenxarch.cpp Set compGeneratingEpilog/compGeneratingProlog during epilog/funclet prolog/epilog generation; update asserts.
src/coreclr/jit/codegenwasm.cpp Make genHomeRegisterParams empty and move homing logic into genHomeRegisterParamsOutsideProlog; set compGeneratingEpilog in epilogs; update asserts in prolog helpers.
src/coreclr/jit/codegenriscv64.cpp Set compiler generation flags for funclet prolog/epilog and main epilog; update asserts.
src/coreclr/jit/codegenloongarch64.cpp Set compiler generation flags for funclet prolog/epilog and main epilog; update asserts.
src/coreclr/jit/codegenlinear.cpp Call WASM register-param homing in the first basic block; adjust poisoning comment.
src/coreclr/jit/codegencommon.cpp Set compGeneratingProlog during main prolog; update several prolog-only asserts to compiler flag.
src/coreclr/jit/codegenarmarch.cpp Set compGeneratingEpilog during main epilog; update prolog asserts.
src/coreclr/jit/codegenarm64.cpp Set compiler generation flags for funclet prolog/epilog; update asserts.
src/coreclr/jit/codegenarm.cpp Set compiler generation flags for funclet prolog/epilog; update asserts.
src/coreclr/jit/codegen.h Add WASM-only declaration for genHomeRegisterParamsOutsideProlog.

Comment thread src/coreclr/jit/emit.h
Comment thread src/coreclr/jit/emitpub.h
Comment thread src/coreclr/jit/codegenlinear.cpp
Copilot AI review requested due to automatic review settings July 2, 2026 23:15

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.

Pull request overview

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

Comment thread src/coreclr/jit/emit.h
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Copilot Code Review

Holistic Assessment

Motivation: This PR reverts PR #128721 ("[RyuJit/WASM] Remove the special prolog handling") and its follow-up fix #130131. The revert is authored by a core maintainer (jkotas), indicating the original change caused problems that warranted a full rollback. Reverting problematic changes is the standard process when issues are discovered post-merge.

Approach: This is a clean mechanical revert of two commits. The resulting code restores the prior approach of using explicit compGeneratingProlog/compGeneratingEpilog booleans on the Compiler class (managed via ScopedSetVariable<bool>), removes the emitter-based emitGeneratingPrologOrFuncletProlog()/emitGeneratingEpilogOrFuncletEpilog() query methods that #128721 introduced, and restores the WASM-specific genHomeRegisterParamsOutsideProlog() pattern that moves parameter homing out of the prolog IG.

Summary: ✅ LGTM. This is a straightforward revert by a core maintainer. The changes are consistent across all affected platforms (x86, ARM, ARM64, LoongArch64, RISC-V64, WASM) and all prolog/epilog/funclet code paths correctly use the ScopedSetVariable pattern to maintain the boolean state.


Detailed Findings

Detailed Findings

✅ Correctness — Consistent state management across all platforms

All prolog entry points (genFnProlog, genFuncletProlog) set compGeneratingProlog = true via ScopedSetVariable<bool>, and all epilog entry points (genFnEpilog, genFuncletEpilog) set compGeneratingEpilog = true the same way. The scoped approach guarantees the flags are reset on function exit. Every platform-specific codegen file (xarch, ARM, ARM64, LoongArch64, RISC-V64, WASM) is updated consistently.

✅ Correctness — WASM prolog overflow workaround restored

The WASM-specific genHomeRegisterParamsOutsideProlog() is properly restored, with the original genHomeRegisterParams becoming an intentionally-empty stub (with a clear comment explaining why). The call site in codegenlinear.cpp correctly invokes it only for block->IsFirst() under #ifdef TARGET_WASM, matching the documented constraint that the WASM prolog can only be one insGroup.

✅ Consistency — Emitter assertions updated correctly

The emitter's emitBegProlog(), emitMarkPrologEnd(), emitEndProlog(), and emitStartExitSeq() all correctly assert the new compiler booleans. The emitSetFrameRangeGCRs assertion is also updated. The removed emitGeneratingPrologOrFuncletProlog() / emitGeneratingEpilogOrFuncletEpilog() methods in emit.cpp and their declarations in emitpub.h are cleanly excised.

✅ Minor cleanup — igData/emitCurIG initialization changes

The move of ig->igData = nullptr to the end of emitInitIG (after the debug-only block) and the removal of emitCurIG = nullptr from the emitter destructor are benign ordering/cleanup changes that don't affect correctness.

💡 Observation — Comment wording in codegenlinear.cpp

The comment for genPoisonFrame was updated from "might use a helper call that kills argument regs" to "might make the prolog too large" — this better describes the actual constraint (prolog size), which is consistent with the WASM comment above it.

Note

This review was generated by GitHub Copilot.

Note

🔒 Integrity filter blocked 1 item

The following item was blocked because it doesn't meet the GitHub integrity level.

  • #128721 issue_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".

To allow these resources, lower min-integrity in your GitHub frontmatter:

tools:
  github:
    min-integrity: approved  # merged | approved | unapproved | none

Generated by Code Review for issue #130148 · ● 54.5M ·

@jkotas jkotas closed this Jul 3, 2026
@jkotas
jkotas deleted the revert-128721-Emit-PrologIG-Exec branch July 3, 2026 18:42
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-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.

ILC failing with exit code 139 for risc-v build

4 participants