eclipse-omr / omr

Eclipse OMR™ Cross platform components for building reliable, high performance language runtimes
http://www.eclipse.org/omr
Other
950 stars 397 forks source link

What are the semantics of fmax/fmin/dmax/dmin for signed zeros? #5256

Open aviansie-ben opened 4 years ago

aviansie-ben commented 4 years ago

When the semantics of the [fd]max and [fd]min opcodes were being discussed with respect to NaN values in https://github.com/eclipse/omr/pull/5121#issuecomment-621395193, it was decided that they should follow the semantics of Java's Math.{max,min} methods and thus should return NaN whenever either of their inputs is a NaN. In addition to that, these methods also require that min(-0.0, +0.0) = min(+0.0, -0.0) = -0.0 [1] and max(-0.0, +0.0) = max(+0.0, -0.0) = +0.0 [2]. This also matches the behaviour specified by WebAssembly [3] and JavaScript [4]. However, this is not currently a case handled by any of the codegens nor by the simplifier.

Should our floating-point max/min opcodes match this behaviour and handle this case? If so, how should we test this in the Tril tests given that -0.0 == +0.0 returns true?

[1] https://docs.oracle.com/javase/8/docs/api/java/lang/Math.html#min-float-float- [2] https://docs.oracle.com/javase/8/docs/api/java/lang/Math.html#max-float-float- [3] https://webassembly.github.io/spec/core/exec/numerics.html#op-fmin [4] https://tc39.es/ecma262/#sec-math.max

janvrany commented 4 years ago

Good point!

If so, how should we test this in the Tril tests given that -0.0 == +0.0 returns true?

Do you mean how make ASSERT_EQ(+0.0, -0.0) to fail? If so, I think (hope) we can extend my hack to handle ASSERT_EQ(NaN, NaN) (see 56099575) to handle this case as well.

aviansie-ben commented 4 years ago

Do you mean how make ASSERT_EQ(+0.0, -0.0) to fail?

Yes. One thing I'm unsure of, though, is the impact that doing this would have on the other floating-point tests. I'm not sure if all of our floating-point opcodes differentiate between results of -0.0 and +0.0, so this behaviour may need to be limited to certain tests.