Add MCS verb to dump jit flags histogram #48281
Conversation
Useful for determining what sorts of jit compilations are found in an SPMI collection.
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
@BruceForstall PTAL Sample output: |
| @@ -0,0 +1,111 @@ | |||
| // | |||
There was a problem hiding this comment.
question: looks like files in ToolBox/superpmi have their own license header, the standard one is:
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
is it expected?
There was a problem hiding this comment.
Thanks for catching that.
I think we need to redo all the license headers in these files. cc @BruceForstall
| //---------------------------------------------------------- | ||
| // verbJitFlags.h - verb that prints sumary of jit flags values | ||
| //---------------------------------------------------------- | ||
| #ifndef _verbJitFlags |
There was a problem hiding this comment.
do we have a preference between #pragma one and ifndef for new files?
There was a problem hiding this comment.
I think ifndef is preferable; it doesn't depend on support for pragma once in compiler.
There was a problem hiding this comment.
Looks useful.
Would be nice to add some comments about how CORJIT_FLAGS is platform specific, but MCS is platform agnostic. We get away with this because we currently never re-use a flag bit on another platform.
You could add the x86-specific and arm-specific flags in the same way you do the FEATURE_SIMD flag.
Possibly you could be platform-specific by using repGetExpectedTargetArchitecture()?
What about another view (in this same verb) the the bit->count mapping (for only the bits ever set), e.g.:
bits, count, name
0000000000000000, 497, SPEED_OPT
0000000000000001, 1, SIZE_OPT
0000000000000002, 200, DEBUG_CODE
...
| // | ||
|
|
||
| //---------------------------------------------------------- | ||
| // verbJitFlags.h - verb that prints sumary of jit flags values |
|
|
||
| printf("%016llx,%8u", flag, flagMap.GetItem(index)); | ||
|
|
||
| for (int flagBit = 63; flagBit >= 0; flagBit--) |
There was a problem hiding this comment.
btw, this should probably be a spmi utility function that can also be used by MethodContext::dmpGetJitFlags().
It also seems odd that you use flagBit: 63->0 ; (flag >> flagBit) & 1ull instead of index: 0->63 ; flag & (1ull << index)
There was a problem hiding this comment.
Good point, think I will adopt the model used by SpmiDumpHelper::DumpCorInfoFlag.
I was iterating backwards because I initially thought the higher-numbered flags were "more interesting."
|
Updated to show per-flag data too. Now I suppose you'll want all that sorted... |
|
By the way the above is telling us that the new benchmark run collection is not running with TC enabled. We really ought to fix that if we want the benchmark run collection to reflect the actual codegen seen in the lab. This is a consequence of superpmi.py disabling tiered compilation. I have the trivial local changes undoing that but am not sure if compensating changes need to be made elsewhere (or, should we add yet another option...?). |
|
FWIW, here's my local Pri-1 test run collection, done with tiered pgo enabled (as well as quick jit for loops). Only about 1% of methods make it to Tier1 here, and I wonder if that's the same for non-pgo runs. We may need to think about ways to boost coverage for tier1 and pgo. |
The only reason we selected this option was to make sure that collection doesn't take long time. I will see if there are options in BDN that will let us do the warmup, etc. but fewer actual runs. |
There was a problem hiding this comment.
Looks great!
Now I suppose you'll want all that sorted...
https://en.wikipedia.org/wiki/Perfect_is_the_enemy_of_good
:-)
|
Unrelated x86 test failure in plug.cmd: @dotnet/gc is this a known issue? |
|
At any rate there's a dump file there if anyone wants to take a look. Going to merge this as the changes here are all parts of SPMI. |
|
@AndyAyersMS - I tried a prototype to run benchmarks the way we run in lab. #48390 I then downloaded the produced What additional thing were you setting to get tiered compilation collection working? |
Seems likely tiering is still disabled somehow, despite your efforts to re-enable? Another possibility is that you might need to set |
|
I can try out that.
Wondering what changes were you referring to to enable tiering? |
|
Also, is it possible that the I have triggered another run with QuickJitForLoops flag - https://dev.azure.com/dnceng/internal/_build/results?buildId=998618&view=results |
Should not be the case after #48082. |
Hhm, didn't notice that PR. But in that case, it should have gathered Tiered compilation methods. I am surprised that it shows 0 tiered methods which is suspicious and I doubt adding |
|
@AndyAyersMS the failure you are seeing is not known. do you only see it with your change? |
|
@Maoni0 as far as I know this is a one-off; I haven't tried to repro it or analyze the dump. This PR does not impact the runtime in any way so whatever this failure is, it is not related to this PR's changes... so perhaps some prior change has destabilized things. |
Yes, so nothing changes with QuickJitForLoops, so I am guessing there is something else wrong. |
@AndyAyersMS just making sure, when you say runtime, does that include the jit? eg, does this change any of the generate code/GC reporting? |
ghost
locked as resolved and limited conversation to collaborators
Useful for determining what sorts of jit compilations are found in an SPMI
collection.