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

Enable SuperPMI replay for APX ISA testing #113843

Merged
merged 2 commits into from
Mar 24, 2025

Conversation

BruceForstall
Copy link
Member

Create a new runtime-coreclr superpmi-replay-apx pipeline that
does SuperPMI replay using AltJit with various APX enabling configuration
options set. This enables some amount of APX testing even without
hardware available to run the generated code.

Notes:

  1. Create a new jit-replay-pipeline.yml template to share the
    superpmi-replay pipeline implementation with the existing pipeline.
  2. The existing pipeline is designated the "standard" replay type and
    the new pipeline the "apx" replay type.
  3. The APX pipeline does replays for Windows x64 and Linux x64.
  4. The configurations tested are:
a. "EnableAPX=1"
b. "EnableAPX=1", "EnableApxNDD=1"
c. "EnableAPX=1", "JitStressRex2Encoding=1"
d. "EnableAPX=1", "JitStressPromotedEvexEncoding=1"
e. "EnableAPX=1", "JitStressRegs=4000"
f. "EnableAPX=1", "EnableApxNDD=1", "JitStressRex2Encoding=1", "JitStressPromotedEvexEncoding=1", "JitStressRegs=4000"

All configurations pass --altjit to superpmi.py to set the AltJit variables, as well as
setting "RunAltJitCode=0" which causes all enabled ISAs to be enabled.
5. Change superpmi to no fail altjit compiles when RunAltJitCode=0 is set. With RunAltJitCode=0, the JIT returns CORJIT_SKIPPED. We don't want superpmi to fail these compiles.
6. nit: fixed typo in superpmi of getStingConfigValue => getStringConfigValue

With RunAltJitCode=0, the JIT returns CORJIT_SKIPPED. We don't want
superpmi to fail these compiles.
Create a new `runtime-coreclr superpmi-replay-apx` pipeline that
does SuperPMI replay an AltJit with various APX enabling configuration
options set. This enables some amount of APX testing even without
hardware available to run the generated code.

Notes:
1. Create a new jit-replay-pipeline.yml template to share the
   superpmi-replay pipeline implementation with the existing pipeline.
2. The existing pipeline is designated the "standard" replay type and
   the new pipeline the "apx" replay type.
3. The APX pipeline does replays for Windows x64 and Linux x64.
4. The configurations tested are:
```
a. "RunAltJitCode=0", "EnableAPX=1", "EnableApxNDD=1"
b. "RunAltJitCode=0", "EnableAPX=1", "JitStressRex2Encoding=1"
c. "RunAltJitCode=0", "EnableAPX=1", "JitStressPromotedEvexEncoding=1"
d. "RunAltJitCode=0", "EnableAPX=1", "JitStressRegs=4000"
e. "RunAltJitCode=0", "EnableAPX=1", "EnableApxNDD=1", "JitStressRex2Encoding=1", "JitStressPromotedEvexEncoding=1", "JitStressRegs=4000"
```
@Copilot Copilot bot review requested due to automatic review settings March 24, 2025 15:23
@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 Mar 24, 2025
@BruceForstall
Copy link
Member Author

@anthonycanino @DeepakRajendrakumaran @Ruihan-Yin @kunalspathak PTAL
cc @dotnet/jit-contrib

Copy link
Contributor

@Copilot Copilot AI left a comment

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 introduces a new SuperPMI replay pipeline for APX ISA testing and updates existing pipelines and scripts to support a new "apx" replay type. Key changes include:

  • Adding a new pipeline file (superpmi-replay-apx.yml) configured for APX replays.
  • Updating the shared pipeline template and job definitions to accept a replayType parameter.
  • Modifying the SuperPMI replay setup and replay scripts to handle "standard" and "apx" configurations.

Reviewed Changes

Copilot reviewed 8 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
eng/pipelines/coreclr/templates/jit-replay-pipeline.yml Adds new parameters (including replayType) for pipeline configuration.
eng/pipelines/coreclr/superpmi-replay-apx.yml Introduces a new pipeline definition for APX replays.
src/coreclr/scripts/superpmi_replay_setup.py Updates argument parsing to require and verify the replay type.
src/coreclr/scripts/superpmi_replay.py Adjusts configuration logic to select between standard and APX settings.
eng/pipelines/coreclr/templates/superpmi-replay-job.yml Updates job naming and artifact retrieval to incorporate replayType.
eng/pipelines/common/templates/runtimes/send-to-helix-step.yml Adds a new parameter for passing the replay type to Helix.
eng/pipelines/coreclr/superpmi-replay.yml Updates the pipeline to extend the new jit-replay-pipeline template with replayType.
eng/pipelines/coreclr/templates/run-superpmi-replay-job.yml Revises the setup call and variable propagation to include replayType.
Files not reviewed (6)
  • src/coreclr/scripts/superpmi-replay.proj: Language not supported
  • src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp: Language not supported
  • src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h: Language not supported
  • src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp: Language not supported
  • src/coreclr/tools/superpmi/superpmi/jitinstance.cpp: Language not supported
  • src/coreclr/tools/superpmi/superpmi/jitinstance.h: Language not supported
Comments suppressed due to low confidence (1)

src/coreclr/scripts/superpmi_replay.py:22

  • [nitpick] Using the argument name "-type" shadows the Python built-in function 'type'. Consider renaming it to "-replayType" (and updating references accordingly) to improve clarity.
parser.add_argument("-type", required=True, help="Type of replay (standard, apx)")

@anthonycanino
Copy link
Contributor

I think we can fold c and d into a single configuration, and eventually may want to do away with them as separate stress modes entirely. The stress modes as separate variables was mostly due to implementation purposes.

@BruceForstall
Copy link
Member Author

I think we can fold c and d into a single configuration, and eventually may want to do away with them as separate stress modes entirely. The stress modes as separate variables was mostly due to implementation purposes.

It wasn't clear to me how useful the independent stress variables are, or if it's valuable/necessary to test them independently. As the stress variables change/merge, we can adjust the testing. E.g., I presume we'd add JitEnableApxConditionalChaining=1 after #111072 is merged.

@BruceForstall
Copy link
Member Author

Contributes to #110672

Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

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

Did you get a chance to do a dry run with this pipeline?
LGTM otherwise.

@BruceForstall
Copy link
Member Author

Did you get a chance to do a dry run with this pipeline? LGTM otherwise.

I tested both the "standard" (existing) replay pipeline and the "apx" replay pipeline (by re-using the existing AzDO pipeline) before submitting this PR. After this is merged, I'll get a separate "apx" pipeline created and configured.

@BruceForstall
Copy link
Member Author

/ba-g Unrelated test failures

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

4 participants