Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: build pred lists before instrumentation #81288

Merged

Conversation

AndyAyersMS
Copy link
Member

@AndyAyersMS AndyAyersMS commented Jan 27, 2023

Move pred list building to just before instrumentation (and just after importation -- we are getting very close to the front of the phase list now).

The block and edge count instrumenters were both using cheap preds to keep track of some relocated count probes. Revise this so they can use the regular pred lists. Also rework both approaches so their RelocateProbes methods are fairly similar and perhaps could be unified one day.

Contributes to #80193.

Diffs

Move pred list building to just before instrumentation (and just after
importation -- we are getting very close to the front of the phase list now).

The block and edge count instrumenters were both using cheap preds to keep
track of some relocated count probes. Revise this so they can use the regular
pred lists. Also rework both approaches so their `RelocateProbes` methods are
fairly similar and perhaps could be unified one day.

Contributes to dotnet#80193.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jan 27, 2023
@ghost ghost assigned AndyAyersMS Jan 27, 2023
@ghost
Copy link

ghost commented Jan 27, 2023

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

Issue Details

Move pred list building to just before instrumentation (and just after importation -- we are getting very close to the front of the phase list now).

The block and edge count instrumenters were both using cheap preds to keep track of some relocated count probes. Revise this so they can use the regular pred lists. Also rework both approaches so their RelocateProbes methods are fairly similar and perhaps could be unified one day.

Contributes to #80193.

Author: AndyAyersMS
Assignees: AndyAyersMS
Labels:

area-CodeGen-coreclr

Milestone: -

@AndyAyersMS
Copy link
Member Author

@EgorBo PTAL
cc @dotnet/jit-contrib

For some context -- both block and edge count instrumentors were not only building cheap preds, but hacking the cheap pred lists to convey information from the RelocateProbes analysis to the actual instrumentation.

I obviously couldn't hack the regular pred lists the same way, so had to find different approaches. The result is simpler overall, especially on the edge probe side where the approach I added (just recently) was overly convoluted.

Some minimal diffs expected for optimized methods with edge probes, mainly because cheap preds did not have any ordering guarantees, and some of the edge instrumentation is sensitive to the order in which preds are enumerated.

@AndyAyersMS
Copy link
Member Author

Failure is some kind of memory corruption on an arm test run. Almost certainly unrelated.

Assert failure(PID 23 [0x00000017], Thread: 44 [0x002c]): GCHeapUtilities::GetGCHeap()->IsInFrozenSegment(obj)
    File: /__w/1/s/src/coreclr/vm/syncblk.cpp Line: 2048
    Image: /root/helix/work/correlation/dotnet

@AndyAyersMS
Copy link
Member Author

/azp run runtime-coreclr jitstress

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@AndyAyersMS
Copy link
Member Author

jitstress failure is #81324

@AndyAyersMS AndyAyersMS merged commit b6935d3 into dotnet:main Jan 30, 2023
@AndyAyersMS AndyAyersMS mentioned this pull request Feb 1, 2023
44 tasks
@ghost ghost locked as resolved and limited conversation to collaborators Mar 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR 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.

2 participants