Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2890613003:
MIPS: Add tests to be skipped because of NaN representation on MIPS (Closed)
|
Created:
3 years, 7 months ago by predrag.rudic Modified:
3 years, 7 months ago CC:
v8-reviews_googlegroups.com Target Ref:
refs/heads/master Project:
v8 Visibility:
Public. |
DescriptionMIPS: Add tests to be skipped because of NaN representation on MIPS
BUG=
Review-Url: https://codereview.chromium.org/2890613003
Cr-Commit-Position: refs/heads/master@{#45451}
Committed: https://chromium.googlesource.com/v8/v8/+/eb5f854950ec4d47b8e81aacbad8692ca8416618
Patch Set 1 #
Total comments: 3
Patch Set 2 : Corrections #
Total comments: 2
Patch Set 3 : Tests tests/conversions don't fail in simulator. #Messages
Total messages: 26 (17 generated)
Description was changed from ========== MIPS: Add tests to be scipped because of NaN representation on MIPS BUG= ========== to ========== MIPS: Add tests to be scipped because of NaN representation on MIPS BUG= ==========
predrag.rudic@imgtec.com changed reviewers: + ahaas@chromium.org, machenbach@chromium.org
PTAL
https://codereview.chromium.org/2890613003/diff/1/test/wasm-spec-tests/wasm-s... File test/wasm-spec-tests/wasm-spec-tests.status (right): https://codereview.chromium.org/2890613003/diff/1/test/wasm-spec-tests/wasm-s... test/wasm-spec-tests/wasm-spec-tests.status:7: # These tests fail because mips does not support the correct NaN bit patterns. Could you maybe make a separate toplevel section for all those, since they have all the condition in common? https://codereview.chromium.org/2890613003/diff/1/test/wasm-spec-tests/wasm-s... test/wasm-spec-tests/wasm-spec-tests.status:8: 'tests/float_misc': [PASS, ['arch == mipsel or arch == mips64el arch == mips or arch == mips64', SKIP]], there's an "or" missing I guess...
PTAL https://codereview.chromium.org/2890613003/diff/1/test/wasm-spec-tests/wasm-s... File test/wasm-spec-tests/wasm-spec-tests.status (right): https://codereview.chromium.org/2890613003/diff/1/test/wasm-spec-tests/wasm-s... test/wasm-spec-tests/wasm-spec-tests.status:7: # These tests fail because mips does not support the correct NaN bit patterns. On 2017/05/17 20:45:02, Michael Achenbach wrote: > Could you maybe make a separate toplevel section for all those, since they have > all the condition in common? Acknowledged.
Description was changed from ========== MIPS: Add tests to be scipped because of NaN representation on MIPS BUG= ========== to ========== MIPS: Add tests to be skipped because of NaN representation on MIPS BUG= ==========
lgtm
ivica.bogosavljevic@imgtec.com changed reviewers: + ivica.bogosavljevic@imgtec.com
https://codereview.chromium.org/2890613003/diff/20001/test/wasm-spec-tests/wa... File test/wasm-spec-tests/wasm-spec-tests.status (right): https://codereview.chromium.org/2890613003/diff/20001/test/wasm-spec-tests/wa... test/wasm-spec-tests/wasm-spec-tests.status:20: 'tests/conversions': [SKIP], Could you please check if these tests are passing on simulator, since simulator uses native NaN and these tests should pass. If that is the case then disable tests on targets only and let it run on simulator
On 2017/05/18 15:13:00, ivica.bogosavljevic wrote: > https://codereview.chromium.org/2890613003/diff/20001/test/wasm-spec-tests/wa... > File test/wasm-spec-tests/wasm-spec-tests.status (right): > > https://codereview.chromium.org/2890613003/diff/20001/test/wasm-spec-tests/wa... > test/wasm-spec-tests/wasm-spec-tests.status:20: 'tests/conversions': [SKIP], > Could you please check if these tests are passing on simulator, since simulator > uses native NaN and these tests should pass. > > If that is the case then disable tests on targets only and let it run on > simulator After offline discussion, we decided to skip NaN failures on simulator as well. The reason is that the behavior on simulator is inconsistent as regard to NaN, somewhere x64 NaNs are generated, elsewhere MIPS NaNs are generated. lgtm
PTAL https://codereview.chromium.org/2890613003/diff/20001/test/wasm-spec-tests/wa... File test/wasm-spec-tests/wasm-spec-tests.status (right): https://codereview.chromium.org/2890613003/diff/20001/test/wasm-spec-tests/wa... test/wasm-spec-tests/wasm-spec-tests.status:20: 'tests/conversions': [SKIP], On 2017/05/18 15:13:00, ivica.bogosavljevic wrote: > Could you please check if these tests are passing on simulator, since simulator > uses native NaN and these tests should pass. > > If that is the case then disable tests on targets only and let it run on > simulator Only tests/conversions don't fail on all platforms in simulator
The CQ bit was checked by predrag.rudic@imgtec.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by predrag.rudic@imgtec.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by predrag.rudic@imgtec.com
The patchset sent to the CQ was uploaded after l-g-t-m from ivica.bogosavljevic@imgtec.com, machenbach@chromium.org Link to the patchset: https://codereview.chromium.org/2890613003/#ps40001 (title: "Tests tests/conversions don't fail in simulator.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495449611049760,
"parent_rev": "457a4a6d7197297827b66701d02704feb55569b9", "commit_rev":
"48cc46bbe61f183d4008cd562f621bcd6d853242"}
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1495449611049760,
"parent_rev": "bdf70aa0e2a272efddbdb7cf710d0cf1739e7fa5", "commit_rev":
"eb5f854950ec4d47b8e81aacbad8692ca8416618"}
Message was sent while issue was closed.
Description was changed from ========== MIPS: Add tests to be skipped because of NaN representation on MIPS BUG= ========== to ========== MIPS: Add tests to be skipped because of NaN representation on MIPS BUG= Review-Url: https://codereview.chromium.org/2890613003 Cr-Commit-Position: refs/heads/master@{#45451} Committed: https://chromium.googlesource.com/v8/v8/+/eb5f854950ec4d47b8e81aacbad8692ca84... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/v8/v8/+/eb5f854950ec4d47b8e81aacbad8692ca84... |
