Skip to content

Commit 3160410

Browse files
committed
winch: Crank the ratchet on tests that can fail
This commit moves all "ignore these outright" tests for Winch and AArch64 to "run the test and expect failure" after recent PRs which removed most of the segfaults/etc which prevented running these tests. One failure remained which was easy enough to fix here: the `cmov` implementation only worked on integer registers, not floating-point registers, leading to a panic. A helper was added for floating-point selects to resolve this panic and get some tests passing. After this fix the `ignore` method is no longer necessary as all tests can be run and be flagged as either pass or fail.
1 parent 5818cec commit 3160410

File tree

4 files changed

+34
-43
lines changed

4 files changed

+34
-43
lines changed

crates/test-util/src/wast.rs

Lines changed: 0 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -409,43 +409,6 @@ impl WastTest {
409409
spec_proposal_from_path(&self.path)
410410
}
411411

412-
/// Returns true for tests that should be ignored (not run) for
413-
/// the given config.
414-
415-
/// The infrastructure for `.wast` tests is designed to enable the
416-
/// execution of tests at all times, however, while compiler
417-
/// backends are in partial development state, it becomes harder
418-
/// to guarantee particular tests will run to completion and
419-
/// return a recoverable error, in some cases tests might segfault
420-
/// making it challenging to assert a failure status.
421-
/// It's recommended to avoid ignoring tests as much as possible, instead
422-
/// it's recommended to add tests to `WastTest::should_fail`.
423-
pub fn ignore(&self, config: &WastConfig) -> bool {
424-
if config.compiler == Compiler::Winch {
425-
if cfg!(target_arch = "aarch64") {
426-
let unsupported = [
427-
// Segfault
428-
"spec_testsuite/conversions.wast",
429-
"spec_testsuite/float_exprs.wast",
430-
"spec_testsuite/int_exprs.wast",
431-
"spec_testsuite/left-to-right.wast",
432-
"spec_testsuite/i32.wast",
433-
"spec_testsuite/traps.wast",
434-
"spec_testsuite/unreachable.wast",
435-
"spec_testsuite/unreached-valid.wast",
436-
"misc_testsuite/component-model/fused.wast",
437-
"misc_testsuite/component-model/strings.wast",
438-
"misc_testsuite/winch/select.wast",
439-
"misc_testsuite/sink-float-but-dont-trap.wast",
440-
"misc_testsuite/winch/use-innermost-frame.wast",
441-
];
442-
443-
return unsupported.iter().any(|part| self.path.ends_with(part));
444-
}
445-
}
446-
false
447-
}
448-
449412
/// Returns whether this test should fail under the specified extra
450413
/// configuration.
451414
pub fn should_fail(&self, config: &WastConfig) -> bool {

tests/wast.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,10 +111,6 @@ fn main() {
111111
fn run_wast(test: &WastTest, config: WastConfig) -> anyhow::Result<()> {
112112
let test_config = test.config.clone();
113113

114-
if test.ignore(&config) {
115-
return Ok(());
116-
}
117-
118114
// Determine whether this test is expected to fail or pass. Regardless the
119115
// test is executed and the result of the execution is asserted to match
120116
// this expectation. Note that this means that the test can't, for example,

winch/codegen/src/isa/aarch64/asm.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -840,6 +840,30 @@ impl Assembler {
840840
});
841841
}
842842

843+
/// If the condition is true, `csel` writes rn to rd. If the
844+
/// condition is false, it writes rm to rd
845+
pub fn fpu_csel(&mut self, rn: Reg, rm: Reg, rd: WritableReg, cond: Cond, size: OperandSize) {
846+
match size {
847+
OperandSize::S32 => {
848+
self.emit(Inst::FpuCSel32 {
849+
rd: rd.map(Into::into),
850+
rn: rn.into(),
851+
rm: rm.into(),
852+
cond,
853+
});
854+
}
855+
OperandSize::S64 => {
856+
self.emit(Inst::FpuCSel64 {
857+
rd: rd.map(Into::into),
858+
rn: rn.into(),
859+
rm: rm.into(),
860+
cond,
861+
});
862+
}
863+
_ => todo!(),
864+
}
865+
}
866+
843867
/// Population count per byte.
844868
pub fn cnt(&mut self, rd: WritableReg) {
845869
self.emit(Inst::VecMisc {

winch/codegen/src/isa/aarch64/masm.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -489,9 +489,17 @@ impl Masm for MacroAssembler {
489489
dst: WritableReg,
490490
src: Reg,
491491
cc: IntCmpKind,
492-
_size: OperandSize,
492+
size: OperandSize,
493493
) -> Result<()> {
494-
self.asm.csel(src, dst.to_reg(), dst, Cond::from(cc));
494+
match (src.class(), dst.to_reg().class()) {
495+
(RegClass::Int, RegClass::Int) => self.asm.csel(src, dst.to_reg(), dst, Cond::from(cc)),
496+
(RegClass::Float, RegClass::Float) => {
497+
self.asm
498+
.fpu_csel(src, dst.to_reg(), dst, Cond::from(cc), size)
499+
}
500+
_ => return Err(anyhow!(CodeGenError::invalid_operand_combination())),
501+
}
502+
495503
Ok(())
496504
}
497505

0 commit comments

Comments
 (0)