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

Fix RISC-V Packed SIMD intrinsics compilation #1674

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

SpriteOvO
Copy link
Contributor

@SpriteOvO SpriteOvO commented Nov 16, 2024

Fixes rust-lang/rust#129593.

Currently the P extension is still unratified, so there is no support for it in the upstream LLVM for now, and thus no LLVM built-in functions or serialization of instructions are provided.

This PR implements quick fixes to some of the instructions to fix them causing compilation failures (see issue rust-lang/rust#129593). Calling it a "quick fix" because ideally we could replace all .insn with LLVM built-in functions in the future when LLVM supports the P extension.

This PR also adds assert_instr(unknown) to each function so that we can at least make sure they compile. Since there is no serialization yet, the stdout of assert_instr is like

---- core_arch::riscv_shared::p::assert_kabs16_unkno1wn stdout ----
disassembly for stdarch_test_shim_kabs16_unkno1wn: 
         0: <unknown>
         1: ret
thread 'core_arch::riscv_shared::p::assert_kabs16_unkno1wn' panicked at crates/stdarch-test/src/lib.rs:172:9:
failed to find instruction `unkno1wn` in the disassembly
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    core_arch::riscv_shared::p::assert_kabs16_unkno1wn

So I left "unknown" here, also with the idea that if LLVM upstream provides support for the P extension at some point in the future, we can know in time and then update our implementation.

r? @workingjubilee
CC @Amanieu

@rustbot
Copy link
Collaborator

rustbot commented Nov 16, 2024

Failed to set assignee to workingjubilee: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@rustbot
Copy link
Collaborator

rustbot commented Nov 16, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @workingjubilee (or someone else) some time within the next two weeks.

@SpriteOvO
Copy link
Contributor Author

r? @Amanieu

@Amanieu
Copy link
Member

Amanieu commented Nov 26, 2024

This shouldn't be checking for unknown: that will just cause the tests to break when the disassembler is updated in the future. It's fine to leave them without instruction tests for now.
On second thought, that's probably fine, it will serve as a reminder to update the instruction tests.

@SpriteOvO
Copy link
Contributor Author

Rebased.

@Amanieu Amanieu merged commit 1f3b4ef into rust-lang:master Nov 27, 2024
54 checks passed
@SpriteOvO SpriteOvO deleted the rv-fix-p-ext branch November 27, 2024 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RISC-V Extension P intrinsics use invalid instruction format
3 participants