Skip to content

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Jul 11, 2017

This is a workaround for #42778, which was git-bisected to #40454's
optimizations to mem::swap, later moved to ptr in #42819. Natively
compiled rustc couldn't even compile stage1 libcore on powerpc64 and
s390x, but they work fine without this repr(simd). Since powerpc64le
works OK, it seems probably related to being big-endian.

The underlying problem is not yet known, but this at least makes those
architectures functional again in the meantime.

cc @arielb1

This is a workaround for rust-lang#42778, which was git-bisected to rust-lang#40454's
optimizations to `mem::swap`, later moved to `ptr` in rust-lang#42819.  Natively
compiled rustc couldn't even compile stage1 libcore on powerpc64 and
s390x, but they work fine without this `repr(simd)`.  Since powerpc64le
works OK, it seems probably related to being big-endian.

The underlying problem is not yet known, but this at least makes those
architectures functional again in the meantime.

cc @arielb1
@arielb1
Copy link
Contributor

arielb1 commented Jul 11, 2017

Nice find! I'm not sure why repr(simd) would break things so horribly - it should not have a "semantic" effect by itself. If you can come up with a "minified" example in which swap breaks that would be amazing. There might be a bug with our ABI code that is worth finding?

Maybe I'll try to look at the LLVM IR differences - I don't think there should be any.

@arielb1
Copy link
Contributor

arielb1 commented Jul 11, 2017

I'll give this an r+ for now to get things working. I am not sure on that swap PR on non-x86 architectures anyway.

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 11, 2017

📌 Commit 77bd4dc has been approved by arielb1

@arielb1
Copy link
Contributor

arielb1 commented Jul 11, 2017

BTW, the "broken" swap seemed to work on a ppc64 qemu (on which building the compiler failed). It would be really nice if we could find out the brokenness.

@arielb1 arielb1 added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jul 11, 2017
@cuviper
Copy link
Member Author

cuviper commented Jul 11, 2017

Yeah, I'm not satisfied with this long-term -- just wanted to make sure to get something in place before the next beta branch. I've tried guessing a few minimal scenarios and not seen any difference in codegen, so we'll have to actually track down which parts are different in the full build.

@bors
Copy link
Collaborator

bors commented Jul 12, 2017

⌛ Testing commit 77bd4dc with merge b9cde66ebe43aa342de8979dbc3e800e6ba3b75c...

@bors
Copy link
Collaborator

bors commented Jul 12, 2017

💔 Test failed - status-travis

@Mark-Simulacrum
Copy link
Member

@bors retry -- android spurious failure, hopefully fixed by clearing caches (I think this started too late, possibly)

@Mark-Simulacrum
Copy link
Member

@bors rollup

@shepmaster
Copy link
Member

Restarted the tests - looks like we ran out of disk space.

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jul 15, 2017
Disable big-endian simd in swap_nonoverlapping_bytes

This is a workaround for rust-lang#42778, which was git-bisected to rust-lang#40454's
optimizations to `mem::swap`, later moved to `ptr` in rust-lang#42819.  Natively
compiled rustc couldn't even compile stage1 libcore on powerpc64 and
s390x, but they work fine without this `repr(simd)`.  Since powerpc64le
works OK, it seems probably related to being big-endian.

The underlying problem is not yet known, but this at least makes those
architectures functional again in the meantime.

cc @arielb1
bors added a commit that referenced this pull request Jul 15, 2017
Rollup of 8 pull requests

- Successful merges: #43074, #43145, #43159, #43202, #43222, #43228, #43229, #43240
- Failed merges:
@bors bors merged commit 77bd4dc into rust-lang:master Jul 15, 2017
@cuviper cuviper deleted the ptr-swap-simd branch September 26, 2017 06:40
bors added a commit that referenced this pull request May 10, 2019
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants