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.
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
f21df70 to
e1e61b6
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. |
| # 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?
|
One of the mips experts mind taking a look? @Gelbpunkt @chenx97 you've been active on a recent mips-related PR |
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