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.

Sign up for GitHub

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

Jump to bottom

More Altivec Intrinsics #43492

Merged
merged 8 commits into from
Jul 29, 2017
Merged

More Altivec Intrinsics #43492

merged 8 commits into from
Jul 29, 2017

Conversation

lu-zero
Copy link
Contributor

@lu-zero lu-zero commented Jul 26, 2017

No description provided.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@lu-zero
Copy link
Contributor Author

lu-zero commented Jul 27, 2017

The failure seems again unrelated to my changeset...

@nikomatsakis
Copy link
Contributor

Hmm, it does, but it's not something we've seen before, and it seems plausible that your change could have caused it. Not sure what to do.

@alexcrichton alexcrichton added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 27, 2017
@alexcrichton
Copy link
Member

The segfaults here look related to this PR (albeit roundaboutly) because it's only segfaulting on a specific powerpc test. My guess is that the modifications of POWERPC_WHITELIST is tickling just the wrong areas of LLVM, so it's likely worth debugging locally to see what's up.

@lu-zero
Copy link
Contributor Author

lu-zero commented Jul 28, 2017

Probably ade5ead whitelists features that aren't present in that version of llvm.

@alexcrichton
Copy link
Member

Looks like CI may still have errors?

@lu-zero
Copy link
Contributor Author

lu-zero commented Jul 28, 2017

My fix works only with llvm-4, llvm-3.7 has the feature available as non-NULL-terminated C arrays.

I can add the hack of hardcoding the list sizes for the 3.7 and the 4.0 API to avoid a feature regression or just ifdef the 4.0 code and always return 0 otherwise.

@lu-zero
Make LLVMRustHasFeature more robust
c471020 
The function should accept feature strings that old LLVM might not
support.

Simplify the code using the same approach used by
LLVMRustPrintTargetFeatures.

Dummify the function for non 4.0 LLVM and update the tests accordingly.
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Jul 28, 2017

📌 Commit c471020 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Jul 28, 2017

⌛ Testing commit c471020 with merge 676d225bf643d08a120c64f5c3d36958678da3b6...

@bors
Copy link
Contributor

bors commented Jul 29, 2017

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

@bors
Copy link
Contributor

bors commented Jul 29, 2017

👀 Test was successful, but fast-forwarding failed: 422 Update is not a fast forward

@Mark-Simulacrum
Copy link
Member

@bors retry -- homu 422 fast forward failed - #43535

@bors
Copy link
Contributor

bors commented Jul 29, 2017

⌛ Testing commit c471020 with merge 6dd8744...

bors added a commit that referenced this pull request Jul 29, 2017
@bors
Auto merge of #43492 - lu-zero:master, r=alexcrichton
6dd8744 
More Altivec Intrinsics
@bors
Copy link
Contributor

bors commented Jul 29, 2017

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

@bors bors merged commit c471020 into rust-lang:master Jul 29, 2017
cuviper added a commit to cuviper/rust that referenced this pull request Aug 1, 2017
@cuviper
Gate LLVMRustHasFeature on LLVM_RUSTLLVM
881a724 
Commit c471020 in rust-lang#43492 make `LLVMRustHasFeature` "more robust"
by using `getFeatureTable()`.  However, this function is specific to
Rust's own LLVM fork, not upstream LLVM-4.0, so we need to use
`#if LLVM_RUSTLLVM` to guard this call.
@cuviper cuviper mentioned this pull request Aug 1, 2017
Merged
bors added a commit that referenced this pull request Aug 2, 2017
@bors
Auto merge of #43579 - cuviper:extern-llvm-features, r=alexcrichton
f5f58d0 
Gate LLVMRustHasFeature on LLVM_RUSTLLVM

Commit c471020 in #43492 make `LLVMRustHasFeature` "more robust"
by using `getFeatureTable()`.  However, this function is specific to
Rust's own LLVM fork, not upstream LLVM-4.0, so we need to use
`#if LLVM_RUSTLLVM` to guard this call.

Closes #43555.
@hsivonen
Copy link
Member

Taking sse2 out of the target feature list when building with system LLVM interferes with building Firefox with system LLVM-using rustc according to FreeBSD bug 223300.

Considering that this issue is about Altivec, it seems bad for the changeset to have a side effect on sse2 which should always be there for x86_64 and should be there by default for x86 unless a distro or the user specifically opt to target non-SSE2 32-bit x86.

@ranma42 ranma42 mentioned this pull request Nov 22, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@lu-zero @rust-highfive @nikomatsakis @alexcrichton @bors @Mark-Simulacrum @hsivonen @ranma42