MIPS: Fix mina_maxa for proper NaN handling.

MIPS: Fix mina_maxa for proper NaN handling.

Also clean up variable naming in min_max and other tests. Fix class_fmt in mips64 assembler test for proper NaN checking

Committed: https://crrev.com/46fafcd706c0c161013c4904b69af0061d983d6f
Cr-Commit-Position: refs/heads/master@{#30073}

[Djordje.Pesic, 2015-08-06 09:19:05]

djordje.pesic@imgtec.com changed reviewers:
+ akos.palfi@imgtec.com, balazs.kilvady@imgtec.com, ilija.pavlovic@imgtec.com,
paul.lind@imgtec.com

[paul.l..., 2015-08-07 04:48:14]

LGTM, but there are some nits to fix before committing.

The current commit message is a little inaccurate, (there are no changes in this
CL to class_fmt, or min_max (though I asked for naming changes in min_max.), and
the 2nd line seems completely redundant with the title.

I suggest:
MIPS: Fix mina_maxa for proper NaN handling.

Also clean up variable naming in min_max.

https://codereview.chromium.org/1276813004/diff/1/test/cctest/test-assembler-...
File test/cctest/test-assembler-mips.cc (right):

https://codereview.chromium.org/1276813004/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips.cc:1805: const int tableLength = 15;
nit: name capitalization does not conform to style guide. Since it's a const, I
suggest kTableLength

https://codereview.chromium.org/1276813004/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips.cc:1811: const float  fltNaN =
std::numeric_limits<float>::quiet_NaN();
nit: these two names also don't conform. Suggest dbl_nan, and flt_nan. I
actually prefer spelling them out, since 'flt' isn't so mnemonic to me:
float_nan, double_nan.

It would be nice if you fix the similar names in TEST(min_max) as well.

https://codereview.chromium.org/1276813004/diff/1/test/cctest/test-assembler-...
File test/cctest/test-assembler-mips64.cc (right):

https://codereview.chromium.org/1276813004/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips64.cc:1907: const int tableLength = 15;
Same comment as in 32-bit versionL kTableLength, double_nan, and float_nan.

[Djordje.Pesic, 2015-08-07 06:31:43]

https://codereview.chromium.org/1276813004/diff/1/test/cctest/test-assembler-...
File test/cctest/test-assembler-mips.cc (right):

https://codereview.chromium.org/1276813004/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips.cc:1805: const int tableLength = 15;
On 2015/08/07 04:48:14, paul.l... wrote:
> nit: name capitalization does not conform to style guide. Since it's a const,
I
> suggest kTableLength

Done. Also changed tableLength to kTableLength in other assembler tests.

https://codereview.chromium.org/1276813004/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips.cc:1811: const float  fltNaN =
std::numeric_limits<float>::quiet_NaN();
On 2015/08/07 04:48:14, paul.l... wrote:
> nit: these two names also don't conform. Suggest dbl_nan, and flt_nan. I
> actually prefer spelling them out, since 'flt' isn't so mnemonic to me:
> float_nan, double_nan.
> 
> It would be nice if you fix the similar names in TEST(min_max) as well.

Done.

https://codereview.chromium.org/1276813004/diff/1/test/cctest/test-assembler-...
File test/cctest/test-assembler-mips64.cc (right):

https://codereview.chromium.org/1276813004/diff/1/test/cctest/test-assembler-...
test/cctest/test-assembler-mips64.cc:1907: const int tableLength = 15;
On 2015/08/07 04:48:14, paul.l... wrote:
> Same comment as in 32-bit versionL kTableLength, double_nan, and float_nan.

Done. Also changed tableLength to kTableLength in other assembler tests. I added
some changes in class_fmt, actually NaN checks

[paul.l..., 2015-08-07 16:49:39]

LGTM, thanks!

[Djordje.Pesic, 2015-08-07 18:16:12]

The CQ bit was checked by djordje.pesic@imgtec.com

[commit-bot: I haz the power, 2015-08-07 18:16:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1276813004/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1276813004/20001

[commit-bot: I haz the power, 2015-08-07 19:05:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-08-07 19:06:06]

Patchset 2 (id:??) landed as
https://crrev.com/46fafcd706c0c161013c4904b69af0061d983d6f
Cr-Commit-Position: refs/heads/master@{#30073}