Remove AggressiveOpt from CastHelpers.LdelemaRef/StelemRef #90412
Conversation
|
And #90416 clears up the ETW stuff, if it's a valid fix (but it seems too easy 😄 ... we'll see what CI and others have to say) |
Co-authored-by: Koundinya Veluri <kouvel@users.noreply.github.com>
…veopt-casthelpers
|
This seems to improve benchmarks, e.g.: object[] _array = new object[1000];
object _obj = "test";
[Benchmark]
public void Test1()
{
// stelem.ref
_array[1] = _obj;
_array[2] = _obj;
}
[Benchmark]
public bool Test2()
{
// CastHelpers.IsInstanceOf*
return _obj is IDisposable
or ICloneable
or Program;
}
I don't know why
Hopefully, the difference is even better on arm64 (don't have any device around currently to test) |
|
/azp list |
This comment was marked as resolved.
This comment was marked as resolved.
|
/azp run runtime-coreclr pgo, runtime-coreclr libraries-pgo |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run runtime-coreclr pgo, runtime-coreclr libraries-pgo |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run runtime-coreclr pgo, runtime-coreclr libraries-pgo |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
@kouvel this is ready for the 2nd round 🙂 all PGO pipelines (with TC=1) look green. Benchmarks: #90412 (comment) |
Possibly because
Given that it was doing a direct call previously also, I wonder if there's a possibility that it could do a direct call to any tier including tier 0 (which would probably be r2r'd code, which may be ok perhaps) or an instrumented tier. Wonder if this should also be specialized for the other tiers to have it do an indirect call instead. |
Like you mentioned, it had AggressiveOptimization attribute on it so presumably it was fine? Going to merge this one into 9.0 now, will see soon perf impact on microbenchmarks (my local runs show improvemnets) The outerloop failure is #90593 |
|
With this change, empty console app (without (this PR removed these two cast helpers from this list + several PRs recently got rid of |
Yea I meant after this change, there would be R2R'ed code after the first call (AggressiveOptimization prevents R2R code to be generated), an intermediate code version with profiling instrumentation with TieredPGO, and finally the tier 1 code. The precode target I imagine would point to those things along the way and returning the precode's target in the fallback path may mean that there is a possibility for there to be direct calls to non-final tiers in some cases. It is probably very rare though. Solving it would probably require taking the lock every time to check the tier. Also similarly for the other cast helpers that don't have AggressiveOptimization. |
Although there may be a shortcut to avoid the lock in startup cases where the helper method hasn't yet been called (maybe the precode target is not set to a native entry point). |
I do not think there is any code that would short-circuit the precode. If the JIT gets a pointer to precode, it is going to call it. |
Ah ok, I misread the code, the fallback path would always be an indirect call and that would be fine. Thanks! |
|
Arm64 codegen diff: object[] _array = new object[1000];
object _obj = "test";
[Benchmark]
public void Test1()
{
_array[1] = _obj;
}
[Benchmark]
public bool Test2()
{
return _obj is IDisposable;
} |
ghost
locked as resolved and limited conversation to collaborators
These two methods show up even for an empty app in
JitDisasmSummary. The idea that we can treat them as normal managed methods and convert to direct calls once they reach Tier1 naturally (or FullOpts with TC=0).Almost the final step (except the ETW stuff) to make empty apps jitting-free 🙂 (well, except the
Main)