From 1c75f7de7e5f93373da34182a8729ace838ef7bd Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Wed, 15 Jul 2020 11:49:21 -0700 Subject: Interpreter: Don't change NaN bits when dividing by 1 (#2958) It's valid to change NaN bits in that case per the wasm spec, but if we do so then fuzz testcases will fail on the optimization of nan:foo / 1 => nan:foo That is, it is ok to leave the bits as they are, and if we do that then we are consistent with the simple and valid optimization of removing a divide by 1. Found by the fuzzer - looks like on x64 on some float32 NaNs, the bits will actually change (see the testcase). I've seen this on two machines consistently, so it's normal apparently. Disable an old wasm spectest that has been updated in upstream anyhow, but the new test here is even more strict and verifies the interpreter literally changes no bits. --- src/wasm/literal.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'src') diff --git a/src/wasm/literal.cpp b/src/wasm/literal.cpp index 29d047723..f0a065c45 100644 --- a/src/wasm/literal.cpp +++ b/src/wasm/literal.cpp @@ -868,6 +868,18 @@ Literal Literal::div(const Literal& other) const { case FP_INFINITE: // fallthrough case FP_NORMAL: // fallthrough case FP_SUBNORMAL: + // Special-case division by 1. nan / 1 can change nan bits per the + // wasm spec, but it is ok to just return that original nan, and we + // do that here so that we are consistent with the optimization of + // removing the / 1 and leaving just the nan. That is, if we just + // do a normal divide and the CPU decides to change the bits, we'd + // give a different result on optimized code, which would look like + // it was a bad optimization. So out of all the valid results to + // return here, return the simplest one that is consistent with + // optimization. + if (rhs == 1) { + return Literal(lhs); + } return Literal(lhs / rhs); default: WASM_UNREACHABLE("invalid fp classification"); @@ -896,6 +908,10 @@ Literal Literal::div(const Literal& other) const { case FP_INFINITE: // fallthrough case FP_NORMAL: // fallthrough case FP_SUBNORMAL: + // See above comment on f32. + if (rhs == 1) { + return Literal(lhs); + } return Literal(lhs / rhs); default: WASM_UNREACHABLE("invalid fp classification"); -- cgit v1.2.3