1041648 - Implement float32x4 clamp test case

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[:ProgramFOX]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release)
Build ID: 20140605174243

Steps to reproduce:

I looked at the test case for the SIMD float32x4 clamp method, in /js/src/tests/ecma_6/TypedObject/simd.


Actual results:

The content of that file was equal to the content of the float32x4 add method test case.


Expected results:

So, a real clamp test case should be implemented now.

Comment 1

[:ProgramFOX]

Attachment: bug-1041648-float32x4-clamp-testcase.diff

Proposed patch for this bug. Test case data taken from the SIMD polyfill, from ecmascript_simd_tests.js.

Comment 2

[Luke Wagner [:luke]]

Comment on attachment 8459706 [details] [diff] [review]
bug-1041648-float32x4-clamp-testcase.diff

Probably want Benjamin for this one.

Comment 3

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8459706 [details] [diff] [review]
bug-1041648-float32x4-clamp-testcase.diff

Review of attachment 8459706 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks a lot for doing this! Could you add just one test case please?

::: js/src/tests/ecma_6/TypedObject/simd/float32x4clamp.js
@@ +7,5 @@
>  
>  function test() {
>    print(BUGNUMBER + ": " + summary);
>  
>    // FIXME -- Bug 948379: Amend to check for correctness of border cases.

Could you remove this comment and add a test that effectively checks for float32 semantics?

Something like the same test as you have, but at some point you'd have a number which cannot be represented precisely as a float32 (e.g. 13.37), and in the assertEq, you'd test for Math.fround(13.37), which is the float32 rounding of 13.37.

Comment 4

[:ProgramFOX]

Attachment: Added float32x4 clamp test case

Done, I have replaced my previous test case with this one containing border cases.

Comment 5

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8460123 [details] [diff] [review]
Added float32x4 clamp test case

Review of attachment 8460123 [details] [diff] [review]:
-----------------------------------------------------------------

Great, thanks a lot! I will push it to tryserver for you.

Comment 6

[Benjamin Bouvier [:bbouvier] (inactive)]

https://tbpl.mozilla.org/?tree=Try&rev=58da82482df3

Comment 7

[Benjamin Bouvier [:bbouvier] (inactive)]

Looks pretty green to me, thanks again!

https://hg.mozilla.org/integration/mozilla-inbound/rev/3bdb961fd6d1

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/3bdb961fd6d1