Skip to content

Add specific formatting string for SSA numbers - #76706

Closed
BruceForstall wants to merge 2 commits into
dotnet:mainfrom
BruceForstall:DistinguishSSANumbers
Closed

Add specific formatting string for SSA numbers#76706
BruceForstall wants to merge 2 commits into
dotnet:mainfrom
BruceForstall:DistinguishSSANumbers

Conversation

@BruceForstall

Copy link
Copy Markdown
Contributor

Format all SSA numbers with the new FMT_SSA string.

Currently, this is defined as ssa%u, which makes it easy to identify a string as an SSA number, as well as making it easy to search for this string throughout the JitDump.

Examples:

N001 [000371] LCL_VAR V01 loc0 u:ssa41 (last use) => $12b {ADD($42, $12a)}
N004 [000369] LCL_VAR V01 loc0 d:ssa42 => $VN.Void
Tree [000368] assigned VN to local var V01/ssa42: $12c {ADD($42, $12b)}
SSA PHI definition: set VN of local V08/ssa3 to $146 {PhiDef($8, $3, $101)} .
SSA PHI definition: set VN of local V01/ssa46 to $147 {PhiDef($1, $2e, $10d)} .
The SSA definition for ByrefExposed (ssa1) at start of BB23 is $c0 {InitVal($41)}
The SSA definition for GcHeap (ssa1) at start of BB23 is $c0 {InitVal($41)}

Format all SSA numbers with the new `FMT_SSA` string.
Currently, this is defined as `ssa%u`, which makes it easy to identify
a string as an SSA number, as well as making it easy to search for this
string throughout the JitDump.
@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 6, 2022
@ghost

ghost commented Oct 6, 2022

Copy link
Copy Markdown

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

Issue Details

Format all SSA numbers with the new FMT_SSA string.

Currently, this is defined as ssa%u, which makes it easy to identify a string as an SSA number, as well as making it easy to search for this string throughout the JitDump.

Examples:

N001 [000371] LCL_VAR V01 loc0 u:ssa41 (last use) => $12b {ADD($42, $12a)}
N004 [000369] LCL_VAR V01 loc0 d:ssa42 => $VN.Void
Tree [000368] assigned VN to local var V01/ssa42: $12c {ADD($42, $12b)}
SSA PHI definition: set VN of local V08/ssa3 to $146 {PhiDef($8, $3, $101)} .
SSA PHI definition: set VN of local V01/ssa46 to $147 {PhiDef($1, $2e, $10d)} .
The SSA definition for ByrefExposed (ssa1) at start of BB23 is $c0 {InitVal($41)}
The SSA definition for GcHeap (ssa1) at start of BB23 is $c0 {InitVal($41)}
Author:BruceForstall
Assignees:BruceForstall
Labels:

area-CodeGen-coreclr

Milestone:-

@BruceForstall

Copy link
Copy Markdown
ContributorAuthor

@AndyAyersMS @dotnet/jit-contrib PTAL

@build-analysisbuild-analysisBot mentioned this pull request Oct 6, 2022
2 tasks

@kunalspathakkunalspathak 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

Copy link
Copy Markdown
Member

I kind of like the current . default though as you note it's not search friendly. How about something like @? I don't think we use it for much right now.

V28@3 seems to flow pretty well.

@kunalspathak

Copy link
Copy Markdown
Contributor

I kind of like the current . default
V28@3 seems to flow pretty well.

May be V28@ssa3 so we can easily search for @ssa throughout the dump file for occurances where SSA is displayed as being tied to variables?

@BruceForstall

Copy link
Copy Markdown
ContributorAuthor

@ is used extensively in the LSRA dumps, which would make it ambiguous.

I'm very biased towards something that is context-free searchable. My peculiar preference is something that works with vi's default settings for the '*' and '#' search commands.

@kunalspathak

Copy link
Copy Markdown
Contributor

@ is used extensively in the LSRA dump

Yep, and that's why I was suggesting to append with ssa, but I see your point.

@AndyAyersMS

Copy link
Copy Markdown
Member

@ is used extensively in the LSRA dumps, which would make it ambiguous.

I'm very biased towards something that is context-free searchable. My peculiar preference is something that works with vi's default settings for the '*' and '#' search commands.

SSA no longer relevant once we get to LSRA, so it seems like this wouldn't be too much of a problem?

Or we could use some other single char that we don't use when dumping flags, like / or ^ or ...?

@BruceForstall

Copy link
Copy Markdown
ContributorAuthor

It looks like we don't use ^ or % or & in the dumps.

^ is problematic for vi / regex because it needs to be escaped

@AndyAyersMS Maybe you can describe more specifically what you don't like about ssa%u or what you would prefer?

We have some similar examples: statements: "STMT%05d", nodes: "N%03d", CSE: "CSE #%02u" (I'd prefer "cse%02u").

Even using "S%u" might be unique enough, but certainly isn't as obvious to the newcomer (to the JIT or phase).

@AndyAyersMS

Copy link
Copy Markdown
Member

It looks like we don't use ^ or % or & in the dumps.

^ is problematic for vi / regex because it needs to be escaped

@AndyAyersMS Maybe you can describe more specifically what you don't like about ssa%u or what you would prefer?

We have some similar examples: statements: "STMT%05d", nodes: "N%03d", CSE: "CSE #%02u" (I'd prefer "cse%02u").

Even using "S%u" might be unique enough, but certainly isn't as obvious to the newcomer (to the JIT or phase).

The ssa number on its own is not very interesting. We should always display it as a suffix of the var, eg V02.1 or similar and not as a separate side annotation like we do now. Then the separator syntax won't matter since the var num will be the main part of the search key.

@BruceForstall

Copy link
Copy Markdown
ContributorAuthor

I'm going to close this for now.

@ghostghost locked as resolved and limited conversation to collaborators Nov 24, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR 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

@BruceForstall@AndyAyersMS@kunalspathak