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

Revert "Disable big-endian simd in swap_nonoverlapping_bytes" #60588

Merged
merged 1 commit into from May 10, 2019

Conversation

Projects
None yet
7 participants
@cuviper
Copy link
Member

commented May 6, 2019

This reverts commit 77bd4dc (#43159).

Issue #42778 was formerly easy to reproduce on two big-endian targets,
powerpc64 and s390x, so we disabled SIMD on this function for all
big-endian targets as a workaround.

I have re-tested this code on powerpc64 and s390x, each with the
bundled LLVM 8 and with external LLVM 7 and LLVM 6, and the problems no
longer appear. So it seems safe to remove this workaround, although I'm
still a little uncomfortable that we never found a root-cause...

Closes #42778.
r? @arielb1

Revert "Disable big-endian simd in swap_nonoverlapping_bytes"
This reverts commit 77bd4dc.

Issue #42778 was formerly easy to reproduce on two big-endian targets,
`powerpc64` and `s390x`, so we disabled SIMD on this function for all
big-endian targets as a workaround.

I have re-tested this code on `powerpc64` and `s390x`, each with the
bundled LLVM 8 and with external LLVM 7 and LLVM 6, and the problems no
longer appear. So it seems safe to remove this workaround, although I'm
still a little uncomfortable that we never found a root-cause...
@sanxiyn

This comment has been minimized.

Copy link
Member

commented May 7, 2019

Do we have a test for this?

@cuviper

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

I don't have a specific test, because I never managed to reduce a reproducer. Back then the native compiler build would consistently fail for me, but @arielb1 found that cross-compiling from x86-64 produced a working compiler.

Now the native build and all libcore tests pass, on both arches with all 3 LLVMs mentioned.

@cuviper

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

The next thing I'm trying is to build cross-compiled rustc with this patch from an x86_64 host, which I will then use as stage0 for new native builds on those targets. If that works too, I think all scenarios will be covered.

@cuviper

This comment has been minimized.

Copy link
Member Author

commented May 8, 2019

Looks good with a cross-compiled stage0 too!

@arielb1

This comment has been minimized.

Copy link
Contributor

commented May 9, 2019

I'm rather busy these days, and will rather have someone else dealing with any fallout this might cause.

r? @michaelwoerister

@michaelwoerister

This comment has been minimized.

Copy link
Contributor

commented May 10, 2019

I don't really know what this is about. Maybe someone from @rust-lang/libs can take over?

@michaelwoerister michaelwoerister removed their assignment May 10, 2019

@alexcrichton

This comment has been minimized.

Copy link
Member

commented May 10, 2019

@bors: r+

@bors

This comment has been minimized.

Copy link
Contributor

commented May 10, 2019

📌 Commit 9a0a87a has been approved by alexcrichton

@bors

This comment has been minimized.

Copy link
Contributor

commented May 10, 2019

⌛️ Testing commit 9a0a87a with merge cff1bdb...

bors added a commit that referenced this pull request May 10, 2019

Auto merge of #60588 - cuviper:be-simd-swap, r=alexcrichton
Revert "Disable big-endian simd in swap_nonoverlapping_bytes"

This reverts commit 77bd4dc (#43159).

Issue #42778 was formerly easy to reproduce on two big-endian targets,
`powerpc64` and `s390x`, so we disabled SIMD on this function for all
big-endian targets as a workaround.

I have re-tested this code on `powerpc64` and `s390x`, each with the
bundled LLVM 8 and with external LLVM 7 and LLVM 6, and the problems no
longer appear. So it seems safe to remove this workaround, although I'm
still a little uncomfortable that we never found a root-cause...

Closes #42778.
r? @arielb1
@bors

This comment has been minimized.

Copy link
Contributor

commented May 10, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: alexcrichton
Pushing cff1bdb to master...

@bors bors added the merged-by-bors label May 10, 2019

@bors bors merged commit 9a0a87a into rust-lang:master May 10, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
homu Test successful
Details

@cuviper cuviper deleted the cuviper:be-simd-swap branch May 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.