Skip to content

Revert "Adding support for the X86Base.Pause intrinsic on Mono (#61707)" - #62157

Merged
agocke merged 1 commit into
dotnet:mainfrom
agocke:revert-61707
Nov 30, 2021
Merged

Revert "Adding support for the X86Base.Pause intrinsic on Mono (#61707)"#62157
agocke merged 1 commit into
dotnet:mainfrom
agocke:revert-61707

Conversation

@agocke

@agockeagocke commented Nov 29, 2021

Copy link
Copy Markdown
Member

This reverts commit 1997bee.

Looks like these tests are still failing after the change.

@agocke

agocke commented Nov 29, 2021

Copy link
Copy Markdown
MemberAuthor

FYI @tannergooding it looks like these tests are still failing in CI, reverting for now

@agocke
agocke requested a review from a teamNovember 29, 2021 21:24
trylek
trylek approved these changes Nov 29, 2021
View reviewed changes

@trylektrylek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@agocke
agocke merged commit 4e9ea99 into dotnet:mainNov 30, 2021
@agocke
agocke deleted the revert-61707 branch November 30, 2021 00:14
@tannergooding

tannergooding commented Nov 30, 2021

Copy link
Copy Markdown
Member

Someone on the Mono team likely needs to join in and help diagnose the issue then. It's not clear what's causing the failure on Mono to me, particularly where the relevant support was added and it was passing locally on my box.

This was referenced Nov 30, 2021
Closed
Closed
Closed
Closed
@imhameed

imhameed commented Dec 1, 2021

Copy link
Copy Markdown
Contributor

Someone on the Mono team likely needs to join in and help diagnose the issue then.

I'll take a look (likely early next week). Is there an open issue tracking this?

@vargaz

vargaz commented Dec 1, 2021

Copy link
Copy Markdown
Contributor

I think the patch is good, but its only implemented when running with --llvm.

@lambdageek

lambdageek commented Dec 1, 2021•
edited
Loading

Copy link
Copy Markdown
Member

Yea, I'd start by un-reverting the patch, but move the issues.targets ExcludeList item so that it doesn't run on mini JIT, interp, non-LLVM AOT, etc.

/cc @fanyang-mono

@ghostghost locked as resolved and limited conversation to collaborators Jan 1, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkoritzinskyjkoritzinskyjkoritzinsky approved these changes
@trylektrylektrylek approved these changes
@imhameedimhameedAwaiting requested review from imhameed
@lambdageeklambdageekAwaiting requested review from lambdageek
@SamMonoRTSamMonoRTAwaiting requested review from SamMonoRT
@vargazvargazAwaiting requested review from vargaz

Assignees

No one assigned

Labels

area-Codegen-meta-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@agocke@tannergooding@imhameed@vargaz@lambdageek@jkoritzinsky@trylek