This repository was archived by the owner on Jan 23, 2023. It is now read-only.
PGI: Load pgort<ver>.dll from the VS native tools env; do not `instal… #15114
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…l` it On Windows, PGO instrumented builds (build.cmd release <arch> pgoinstrument) introduce a runtime dependency on pgort<ver>.dll for instrumented binaries. This DLL is distributed alongside the C++ compiler, and is made available via the native tools environment that ships with Visual Studio. Previously, we were using cmake to find and "install" this binary alongside the product when doing an instrumented build, so that the resulting bin\Product drop is free of any added external dependencies. However, this approach is fragile, and despite a best effort to make the implementation work across multiple VS releases, it already broke with VS 2017. To fix support for pgoinstrument on VS 2017, and to harden the implementation for future releases of VS, I'm removing the custom cmake install logic for the pgort DLL. Instead, we fall back to the officially supported method: load the correct (native tools) environment before invoking any command that uses an instrumented binary. This happens in one place in the build today--loading the JIT to crossgen System.Private.CoreLib.dll. Note that there's still an existing CLI/Setup bug that requires copying the pgort DLL. We're now doing it from within build.cmd, which is not nearly as fragile for this as cmake is. The workaround is also isolated, so when the referenced issue is fixed, the workaround (as documented) can simply be removed.
|
/cc: @dotnet/rap-team Currently our windows release/2.0.0 pgo optimizations are failing because of a failure to load pgort. This ports over a change from master that @dpodder implemented in September. |
brianrob
approved these changes
Nov 20, 2017
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…l` it
On Windows, PGO instrumented builds (build.cmd release
pgoinstrument) introduce a runtime dependency on pgort.dll for
instrumented binaries. This DLL is distributed alongside the C++
compiler, and is made available via the native tools environment that
ships with Visual Studio.
Previously, we were using cmake to find and "install" this binary
alongside the product when doing an instrumented build, so that the
resulting bin\Product drop is free of any added external dependencies.
However, this approach is fragile, and despite a best effort to make the
implementation work across multiple VS releases, it already broke with
VS 2017.
To fix support for pgoinstrument on VS 2017, and to harden the
implementation for future releases of VS, I'm removing the custom cmake
install logic for the pgort DLL. Instead, we fall back to the officially
supported method: load the correct (native tools) environment before
invoking any command that uses an instrumented binary. This happens in
one place in the build today--loading the JIT to crossgen
System.Private.CoreLib.dll.
Note that there's still an existing CLI/Setup bug that requires copying
the pgort DLL. We're now doing it from within build.cmd, which is not
nearly as fragile for this as cmake is. The workaround is also isolated,
so when the referenced issue is fixed, the workaround (as documented)
can simply be removed.