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

Trial reversion of37679 #39335

Merged
merged 1 commit into from
Jul 17, 2020
Merged

Conversation

AndyAyersMS
Copy link
Member

Continuation of #38691

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jul 15, 2020
@kunalspathak
Copy link
Member

@safern - any idea why CI run didn't kick off automatically?

@safern
Copy link
Member

safern commented Jul 15, 2020

No idea. Let's try closing and re-opening.

@safern safern closed this Jul 15, 2020
@safern safern reopened this Jul 15, 2020
@AndyAyersMS AndyAyersMS force-pushed the TrialReversionOf37679 branch from 16775de to 56338b3 Compare July 15, 2020 18:06
@AndyAyersMS
Copy link
Member Author

Rebased so I could manually kick off some outerloop legs.

@AndyAyersMS
Copy link
Member Author

@noahfalk do you know if we run any diagnostics tests with profiler-supplied enter/exit hooks on linux arm32?

Curious if I can find or create a repro for #38230 that does not involve jit stress or gc stress.

@noahfalk
Copy link
Member

@davmason probably knows the answer. We've got some ELT tests but I can't recall if we have the win arm32 specific variant.

@davmason
Copy link
Member

@AndyAyersMS the answer is not as clear cut as I would like. The ELT tests live in the private diagnostic tests repo, we don't run arm32 tests in CI there. We do have some ELT tests that theoretically should run on arm, but since they haven't been running for who knows how long I am not sure how much coverage they will provide. If you're just looking for a way to get an ELT profiler up and running I have an ELT profiler I built to help with the arm64 ELT bringup I could share

@AndyAyersMS
Copy link
Member Author

The bug I'd be trying to repro (#38230) is arm linux so I'd need something that works exactly for that ISA/OS. I can find various profiler samples out there I could potentially adapt but none of them have the right low-level bits for arm/linux.

I suppose an alternative might be some sort of null profiler mode, where the runtime tells the jit to add the ELT hooks and the hooks get invoked at runtime but end up doing nothing as no profiler is registered -- do we have something like that? It would sure make adding a suitable test in our CI simpler....

@davmason
Copy link
Member

There's COMPLUS_TestOnlyEnableSlowELTHooks, I haven't used it personally in a long time and the code that reads it is behind ifdef PROF_TEST_ONLY_FORCE_ELT so I am suspicious if it's still compiling on coreclr.

The sample ELT profiler I have uses slow path ELT so shouldn't require require any changes to run on arm32. I can send it to you

@AndyAyersMS
Copy link
Member Author

@dotnet/jit-contrib I think we should merge this as it should improve our arm32 jitstress/gcstress generally.

At this point it might be simpler to sort out what still fails afterwards.

@davmason davmason mentioned this pull request Jul 17, 2020
@AndyAyersMS
Copy link
Member Author

Let me rerun "runtime" at least to see if it clears up those errors.

@AndyAyersMS
Copy link
Member Author

Still seeing those extensions failures, which were fixed in #39404 and I thought would merge up on rerun. Am going to merge anyways.

@AndyAyersMS AndyAyersMS merged commit 87d8f7d into dotnet:master Jul 17, 2020
@AndyAyersMS AndyAyersMS deleted the TrialReversionOf37679 branch July 17, 2020 23:21
Jacksondr5 pushed a commit to Jacksondr5/runtime that referenced this pull request Aug 10, 2020
@karelz karelz added this to the 5.0.0 milestone Aug 18, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
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.

9 participants