Skip to content

allow passing jit option to superpmi.py replay#48738

Merged
AndyAyersMS merged 1 commit into
dotnet:masterfrom
AndyAyersMS:SuperPmiReplayJitOption
Feb 25, 2021
Merged

allow passing jit option to superpmi.py replay#48738
AndyAyersMS merged 1 commit into
dotnet:masterfrom
AndyAyersMS:SuperPmiReplayJitOption

Conversation

@AndyAyersMS

Copy link
Copy Markdown
Member

No description provided.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 25, 2021
@AndyAyersMS

Copy link
Copy Markdown
Member Author

@BruceForstall PTAL
cc @dotnet/jit-contrib

I am using this to enable extra checking with my locally collected PGO MCH files, as part of tracking down where profiles become inconsistent:

superpmi.py replay -mch_files d:\bugs\spmi-pgo\tiered-pgo-9.mch --jitoption JitProfileChecks=2

@AndyAyersMS

Copy link
Copy Markdown
Member Author

@kunalspathak maybe you can take a look?

@kunalspathak kunalspathak 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.

LGTM

@AndyAyersMS AndyAyersMS merged commit b60a80d into dotnet:master Feb 25, 2021
@AndyAyersMS AndyAyersMS deleted the SuperPmiReplayJitOption branch February 25, 2021 17:30
@BruceForstall

Copy link
Copy Markdown
Contributor

I guess you prefer this to setting an environment variable before invoking the python script? Seems like this feature might be more convenient in some scripting.

For consistency, though, I think the option should only have a single leading - : my understanding of the convention is that a single - switch takes an argument; a double -- does not.

@AndyAyersMS

Copy link
Copy Markdown
Member Author

Ah, did not realize that was the convention, thought maybe -- was for more obscure options.

Do we document in superpmi.py that ambient COMPlus env vars are hoovered up and passed along as jitoption ?

I wasn't sure I could/should count on this.

@BruceForstall

Copy link
Copy Markdown
Contributor

Do we document in superpmi.py that ambient COMPlus env vars are hoovered up and passed along as jitoption ?

superpmi.exe (implementing the host interface) itself reads the environment:

const WCHAR* JitHost::getStringConfigValue(const WCHAR* key)
. It looks in this order:

  1. -jitoption force
  2. stored value in collection
  3. -jitoption
  4. environment

I also realized you only changed replay. You also want to set these for asmdiffs, in which case you need to add them to target_flags (probably):

target_flags = []
. There are a set of flags that get passed to both the "determine asm diffs" command and each "generated textual asm", and you want these to be set for both. Currently, this is only the target and altjit-related commands, so the variable name is a bit specific to altjit, as complus vars are handled separately.

@ghost ghost locked as resolved and limited conversation to collaborators Mar 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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.

3 participants