-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Trial reversion of37679 #39335
Conversation
@safern - any idea why CI run didn't kick off automatically? |
No idea. Let's try closing and re-opening. |
This reverts commit b179e19.
16775de
to
56338b3
Compare
Rebased so I could manually kick off some outerloop legs. |
@davmason probably knows the answer. We've got some ELT tests but I can't recall if we have the win arm32 specific variant. |
@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 |
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.... |
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 |
@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. |
Let me rerun "runtime" at least to see if it clears up those errors. |
Still seeing those extensions failures, which were fixed in #39404 and I thought would merge up on rerun. Am going to merge anyways. |
This reverts commit b179e19.
Continuation of #38691