JIT: Move profile checking back until just before inlining #101011

AndyAyersMS · 2024-04-13T18:18:48Z

Fixes the following areas with proper profile updates:

GDV chaining
instrumentation-introduces flow
OSR step blocks
fgSplitEdge (used by instrumentation)

Adds checking bypasses for:

callfinally pair tails
original method entries in OSR methods

Contributes to #93020

Fixes the following areas with proper profile updates: * GDV chaining * instrumentation-introduces flow * OSR step blocks * fgSplitEdge (used by instrumentation) Adds checking bypasses for: * callfinally pair tails * original method entries in OSR methods Contributes to dotnet#93020

dotnet-policy-service · 2024-04-13T18:19:15Z

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

AndyAyersMS · 2024-04-14T14:22:48Z

@amanasifkhalid PTAL
cc @dotnet/jit-contrib

Modest number of diffs from some of the profile changes.

FYI getting this checking past inlining is going to require some more work. Need to think about it a bit.

AndyAyersMS · 2024-04-14T14:23:41Z

/azp run runtime-coreclr libraries-pgo, runtime-coreclr jitstress, runtime-coreclr pgostress, runtime-coreclr pgo

azure-pipelines · 2024-04-14T14:24:03Z

Azure Pipelines successfully started running 4 pipeline(s).

AndyAyersMS · 2024-04-15T14:37:10Z

Stress shows a number of related failures, so this needs more work.

amanasifkhalid

Preliminary pass LGTM

AndyAyersMS · 2024-04-16T21:33:23Z

/azp run runtime-coreclr libraries-pgo, runtime-coreclr jitstress, runtime-coreclr pgostress, runtime-coreclr pgo

azure-pipelines · 2024-04-16T21:33:29Z

Pull request contains merge conflicts.

AndyAyersMS · 2024-04-16T22:06:44Z

/azp run runtime-coreclr libraries-pgo, runtime-coreclr jitstress, runtime-coreclr pgostress, runtime-coreclr pgo

azure-pipelines · 2024-04-16T22:07:02Z

Azure Pipelines successfully started running 4 pipeline(s).

AndyAyersMS · 2024-04-17T22:50:44Z

The failures (at least pgostress so far) are proving difficult to repro. Starting to worry that having this checking on by default is going to be counterproductive, if we keep seeing a long tail of rare cases that don't get properly handled and are hard to capture/reproduce.

amanasifkhalid · 2024-04-18T15:11:59Z

The failures (at least pgostress so far) are proving difficult to repro. Starting to worry that having this checking on by default is going to be counterproductive, if we keep seeing a long tail of rare cases that don't get properly handled and are hard to capture/reproduce.

I suppose if inlining and block layout are most likely to benefit from a robust profile, and your current work has ensured profiles are consistent for the majority of methods up to inlining, then maybe it makes sense to scale back our broader goals for profile consistency to just up to inlining? We can always just re-run profile repair right before doing layout.

AndyAyersMS · 2024-04-19T18:27:50Z

jit-format is failing because we can't extract the compile commands...

;; linux 
Running: /__w/1/s/src/coreclr/build-runtime.sh x64 Checked configureonly -cmakeargs -[18:02:22] Running: /tmp/tmpk8nyid8a/jitutils/bin/jit-format -a x64 -b Checked -o linux -r /__w/1/s --verbose --projects dll --cross
Formatting jit directory.
Can't find compile_commands.json file. Running configure.
Running: /__w/1/s/src/coreclr/build-runtime.sh x64 Checked configureonly -cmakeargs -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -cross
There was an error running CMake to generate compile_commands.json. Please run build-runtime.sh configureonly


;; windows

Formatting jit directory.
Neither compile_commands.json exists, nor is there a build log. Running CMake to generate compile_commands.json.
Running: D:\a\_work\1\s\src\coreclr\build-runtime.cmd x64 Checked -configureonly -ninja
BUILD: Starting Build at 18:09:12.60
BUILD: Commencing CoreCLR product build
BUILD: Checking prerequisites
BUILD: Using CMake from C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe
BUILD: Number of processor cores 2
        1 file(s) copied.
        1 file(s) copied.
        1 file(s) copied.
BUILD: Commencing build of native components for windows.x64.Checked
BUILD: Using environment: "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\Build\vcvarsall.bat" amd64
**********************************************************************
** Visual Studio 2022 Developer Command Prompt v17.9.4
** Copyright (c) 2022 Microsoft Corporation
**********************************************************************
[vcvarsall.bat] Environment initialized for: 'x64'
BUILD: Regenerating the Visual Studio solution
Calling "D:\a\_work\1\s\eng\native\gen-buildsys.cmd" "D:\a\_work\1\s\src\coreclr" "D:\a\_work\1\s\artifacts\obj\coreclr\windows.x64.Checked" vs2022 x64 windows "-DCMAKE_BUILD_TYPE=Checked" "-DCLR_CMAKE_TARGET_ARCH=x64" "-DCLR_CMAKE_TARGET_OS=windows" "-DCLI_CMAKE_FALLBACK_OS=win10" "-DCLR_CMAKE_PGO_INSTRUMENT=0" "-DCLR_CMAKE_OPTDATA_PATH=" "-DCLR_CMAKE_PGO_OPTIMIZE=0" 
Not searching for unused variables given on the command line.
-- The C compiler identification is MSVC 19.39.33523.0
-- The CXX compiler identification is MSVC 19.39.33523.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.39.33519/bin/Hostx64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.39.33519/bin/Hostx64/x64/cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- The ASM_MASM compiler identification is MSVC
-- Found assembler: C:/Program Files/Microsoft Visual Studio/2022/Enterprise/VC/Tools/MSVC/14.39.33519/bin/Hostx64/x64/ml64.exe
-- Performing Test HAVE_DIRENT_D_TYPE
-- Performing Test HAVE_DIRENT_D_TYPE - Failed
-- Looking for getauxval

Looks like this is getting fixed in #101297.

AndyAyersMS · 2024-04-19T18:34:40Z

/azp run runtime-coreclr libraries-pgo, runtime-coreclr jitstress, runtime-coreclr pgostress, runtime-coreclr pgo, runtime-coreclr libraries-jitstress

azure-pipelines · 2024-04-19T18:35:07Z

Azure Pipelines successfully started running 5 pipeline(s).

AndyAyersMS · 2024-04-20T00:32:55Z

libraries-jitstress and libraries-pgo seem like they may be unrelated.

But pgostress is related. Will keep trying to repro this locally.

AndyAyersMS · 2024-04-20T01:03:05Z

Finally caught one

BB77 and BB80 are mis-weighted (likely just BB80) -- seems like multi-guess GDV is not quite right.

AndyAyersMS · 2024-04-20T01:18:32Z

/azp run runtime-coreclr libraries-pgo, runtime-coreclr jitstress, runtime-coreclr pgostress, runtime-coreclr pgo, runtime-coreclr libraries-jitstress

azure-pipelines · 2024-04-20T01:18:51Z

Azure Pipelines successfully started running 5 pipeline(s).

AndyAyersMS · 2024-04-20T06:04:37Z

@amanasifkhalid think this is ready...

amanasifkhalid

LGTM, thanks for seeing this through!

We briefly discussed not continuing this validation past inlining for now, as inlining and block layout (and hot-cold splitting) are probably the opt passes that benefit most from high-quality profile data, and we can simply re-run profile repair right before the latter to ensure it has a useful profile to work with. Plus, the stress failures these consistency checks have triggered seem to be a pain to diagnose. Are you planning on stopping here for now?

amanasifkhalid · 2024-04-21T23:08:44Z

src/coreclr/jit/indirectcalltransformer.cpp

+                        }
+                    }
+
+                    // No matter what,  the minimum weight is zero


Suggested change

// No matter what, the minimum weight is zero

// No matter what, the minimum weight is zero

amanasifkhalid · 2024-04-21T23:14:32Z

libraries-pgo failure looks unrelated, and SPMI failures are #101070.

AndyAyersMS · 2024-04-22T14:54:43Z

Are you planning on stopping here for now?

I will probably keep pushing on this for the next week or two; would be nice to be able to validate more of the flow optimizations (which generally should have local repairs).

Also will fix the comment issue you noted above.

…1011) Fixes the following areas with proper profile updates: * GDV chaining * instrumentation-introduces flow * OSR step blocks * fgSplitEdge (used by instrumentation) Adds checking bypasses for: * callfinally pair tails * original method entries in OSR methods Contributes to dotnet#93020

dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 13, 2024

dotnet-policy-service bot assigned AndyAyersMS Apr 13, 2024

amanasifkhalid reviewed Apr 15, 2024

View reviewed changes

fix weight of scratch BB

30c0b7c

Merge branch 'main' into BlockWeightConsistency2a

2777a7c

AndyAyersMS added 2 commits April 18, 2024 18:37

fix osr stress issue

22ba6d5

Merge branch 'main' into BlockWeightConsistency2a

9331f10

AndyAyersMS added 2 commits April 19, 2024 18:02

fix multi-guess GDV profile

40d86ed

Merge branch 'main' into BlockWeightConsistency2a

4721483

amanasifkhalid approved these changes Apr 21, 2024

View reviewed changes

AndyAyersMS merged commit aaebdd8 into dotnet:main Apr 22, 2024
170 of 174 checks passed

DrewScoggins mentioned this pull request Apr 30, 2024

[Perf] Windows/x64: 1 Regression on 4/22/2024 7:01:47 PM #101723

Closed

AndyAyersMS mentioned this pull request May 3, 2024

JIT: Flow Graph Modernization and Improved Block Layout #93020

Closed

51 tasks

github-actions bot locked and limited conversation to collaborators May 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JIT: Move profile checking back until just before inlining #101011

JIT: Move profile checking back until just before inlining #101011

AndyAyersMS commented Apr 13, 2024

dotnet-policy-service bot commented Apr 13, 2024

AndyAyersMS commented Apr 14, 2024

AndyAyersMS commented Apr 14, 2024

azure-pipelines bot commented Apr 14, 2024

AndyAyersMS commented Apr 15, 2024

amanasifkhalid left a comment

AndyAyersMS commented Apr 16, 2024

azure-pipelines bot commented Apr 16, 2024

AndyAyersMS commented Apr 16, 2024

azure-pipelines bot commented Apr 16, 2024

AndyAyersMS commented Apr 17, 2024

amanasifkhalid commented Apr 18, 2024 •

edited

Loading

AndyAyersMS commented Apr 19, 2024

AndyAyersMS commented Apr 19, 2024

azure-pipelines bot commented Apr 19, 2024

AndyAyersMS commented Apr 20, 2024

AndyAyersMS commented Apr 20, 2024

AndyAyersMS commented Apr 20, 2024

azure-pipelines bot commented Apr 20, 2024

AndyAyersMS commented Apr 20, 2024

amanasifkhalid left a comment

amanasifkhalid Apr 21, 2024

amanasifkhalid commented Apr 21, 2024

AndyAyersMS commented Apr 22, 2024

	// No matter what, the minimum weight is zero
	// No matter what, the minimum weight is zero

JIT: Move profile checking back until just before inlining #101011

JIT: Move profile checking back until just before inlining #101011

Conversation

AndyAyersMS commented Apr 13, 2024

dotnet-policy-service bot commented Apr 13, 2024

AndyAyersMS commented Apr 14, 2024

AndyAyersMS commented Apr 14, 2024

azure-pipelines bot commented Apr 14, 2024

AndyAyersMS commented Apr 15, 2024

amanasifkhalid left a comment

Choose a reason for hiding this comment

AndyAyersMS commented Apr 16, 2024

azure-pipelines bot commented Apr 16, 2024

AndyAyersMS commented Apr 16, 2024

azure-pipelines bot commented Apr 16, 2024

AndyAyersMS commented Apr 17, 2024

amanasifkhalid commented Apr 18, 2024 • edited Loading

AndyAyersMS commented Apr 19, 2024

AndyAyersMS commented Apr 19, 2024

azure-pipelines bot commented Apr 19, 2024

AndyAyersMS commented Apr 20, 2024

AndyAyersMS commented Apr 20, 2024

AndyAyersMS commented Apr 20, 2024

azure-pipelines bot commented Apr 20, 2024

AndyAyersMS commented Apr 20, 2024

amanasifkhalid left a comment

Choose a reason for hiding this comment

amanasifkhalid Apr 21, 2024

Choose a reason for hiding this comment

amanasifkhalid commented Apr 21, 2024

AndyAyersMS commented Apr 22, 2024

amanasifkhalid commented Apr 18, 2024 •

edited

Loading