Repository navigation
fix mips64el tests - #2260
fix mips64el tests#2260folkertdev wants to merge 12 commits into
mips64el tests#2260Conversation
| let r = f32x4::new( | ||
| 1.0 * 5.0 + 9.0, | ||
| 2.0 * 6.0 + 10.0, | ||
| 3.0 * 7.0 + 11.0, | ||
| 4.0 * 8.0 + 12.0, | ||
| ); |
There was a problem hiding this comment.
this evaluation order of the 3 inputs matches the documentation, so I have no idea why this test was ever written as it was.
|
r? @adamgemmell rustbot has assigned @adamgemmell. Use Why was this reviewer chosen?The reviewer was selected based on:
|
This comment has been minimized.
This comment has been minimized.
f21df70 to
e1e61b6
Compare
This comment has been minimized.
This comment has been minimized.
| # gcc-mips64-linux-gnuabi64 not available in 25.10 | ||
| FROM ubuntu:25.04 |
There was a problem hiding this comment.
Fyi you can get it on later versions by running echo "deb http://archive.ubuntu.com/ubuntu noble main universe" >> /etc/apt/sources.list.d/noble-copies.list before the apt update
| ENV CARGO_TARGET_MIPS64_UNKNOWN_LINUX_GNUABI64_LINKER=mips64-linux-gnuabi64-gcc \ | ||
| CARGO_TARGET_MIPS64_UNKNOWN_LINUX_GNUABI64_RUNNER="qemu-mips64 -L /usr/mips64-linux-gnuabi64" \ | ||
| OBJDUMP=mips64-linux-gnuabi64-objdump No newline at end of file | ||
| CARGO_TARGET_MIPS64_UNKNOWN_LINUX_GNUABI64_RUNNER="qemu-mips64 -cpu Loongson-3A4000 -L /usr/mips64-linux-gnuabi64" \ |
There was a problem hiding this comment.
Maybe add a comment about why the cpu is needed?
There was a problem hiding this comment.
I'm assuming this is because there doesn't seem to be a "max" cpu for qemu-mips64?
There was a problem hiding this comment.
Yeah, from what I understand the CPUs covered by MIPS are quite a broad set, with various breaking changes, so it's not really possible to pick a single max CPU. I just picked one with MSA.
|
One of the mips experts mind taking a look? @Gelbpunkt @chenx97 you've been active on a recent mips-related PR |
This matches LLVM
```
class SRLI_B_DESC : MSA_BIT_SPLAT_DESC_BASE<"srli.b", srl, vsplati8_uimm3,
MSA128BOpnd>;
class SRLI_H_DESC : MSA_BIT_SPLAT_DESC_BASE<"srli.h", srl, vsplati16_uimm4,
MSA128HOpnd>;
class SRLI_W_DESC : MSA_BIT_SPLAT_DESC_BASE<"srli.w", srl, vsplati32_uimm5,
MSA128WOpnd>;
class SRLI_D_DESC : MSA_BIT_SPLAT_DESC_BASE<"srli.d", srl, vsplati64_uimm6,
MSA128DOpnd>;
```
I suspect the old code mistakenly matched these variants
```
class SRLRI_B_DESC : MSA_BIT_X_DESC_BASE<"srlri.b", int_mips_srlri_b, uimm3,
timmZExt3, MSA128BOpnd>;
class SRLRI_H_DESC : MSA_BIT_X_DESC_BASE<"srlri.h", int_mips_srlri_h, uimm4,
timmZExt4, MSA128HOpnd>;
class SRLRI_W_DESC : MSA_BIT_X_DESC_BASE<"srlri.w", int_mips_srlri_w, uimm5,
timmZExt5, MSA128WOpnd>;
class SRLRI_D_DESC : MSA_BIT_X_DESC_BASE<"srlri.d", int_mips_srlri_d, uimm6,
timmZExt6, MSA128DOpnd>;
```
the all-bits-set input is optimized in a way that defeats the tests, so use a non-trivial input
note that the immediate is signed, so we actually have 9 bits for the value
at least for me locally a comment can't be on the FROM line
…on so they are equivalent
…name suggests our assert_instr checks now match the actual clang output, update stdarch-verify to match
e1e61b6 to
1cc796f
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Make
ci/run-docker.shformips64elwork again. The big-endianmips64should also work conceptually, but runs into an endianness bug llvm/llvm-project#236458.For changed
assert_instrs, see https://godbolt.org/z/fz5EExhET. We're not checking for what clang actually emits, and seem to match it.stdarch-verifyneeded some changes, though maybe we can just disable it now that we haveassert_instr?In other cases the test input we provided was some sort of trivial case that got optimized out. So giving slightly different input makes the test pass.
We were also using incorrect immediate widths for some intrinsics, the widths are based on LLVM