Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1778673004:
MIPS: Enable asm-wasm mjsunit tests (skip on arm and mips). (Closed)
|
Created:
4 years, 9 months ago by balazs.kilvady Modified:
4 years, 9 months ago Reviewers:
bradn, Michael Achenbach, bradnelson, danno, machenbach, gergely.kis.imgtec, Jakob Kummerow, akos.palfi.imgtec CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionMIPS: Disable asm-wasm mjsunit tests.
Port 879d254d54eb1d92721c421aebb9a43faff9eb8a
Original commit message:
Landing this again, excluding arm as it fail and arm64
out of caution that other wasm arm64 tests are also disabled.
BUG= https://code.google.com/p/v8/issues/detail?id=4203
TEST=mjsunit/asm-wasm
LOG=N
Committed: https://crrev.com/7b9dca76b8bca685ef95473b795e83227bbf0768
Cr-Commit-Position: refs/heads/master@{#34860}
Patch Set 1 #
Total comments: 2
Messages
Total messages: 24 (13 generated)
balazs.kilvady@imgtec.com changed reviewers: + akos.palfi@imgtec.com, bradnelson@chromium.org, gergely.kis@imgtec.com
bradnelson@google.com changed reviewers: + bradnelson@google.com
https://codereview.chromium.org/1778673004/diff/1/test/mjsunit/mjsunit.status File test/mjsunit/mjsunit.status (right): https://codereview.chromium.org/1778673004/diff/1/test/mjsunit/mjsunit.status... test/mjsunit/mjsunit.status:312: 'wasm/asm-wasm': [PASS, ['arch in [arm, arm64, mips, mipsel, mips64, mips64el]', SKIP]], I'm confused, doesn't this disable asm-wasm for mips?
Description was changed from ========== MIPS: Enable asm-wasm mjsunit tests (skip on arm and mips). Port 879d254d54eb1d92721c421aebb9a43faff9eb8a Original commit message: Landing this again, excluding arm as it fail and arm64 out of caution that other wasm arm64 tests are also disabled. BUG= https://code.google.com/p/v8/issues/detail?id=4203 TEST=mjsunit/asm-wasm LOG=N ========== to ========== MIPS: Enable asm-wasm mjsunit tests (skip on arm and mips). Port 879d254d54eb1d92721c421aebb9a43faff9eb8a Original commit message: Landing this again, excluding arm as it fail and arm64 out of caution that other wasm arm64 tests are also disabled. BUG= https://code.google.com/p/v8/issues/detail?id=4203 TEST=mjsunit/asm-wasm LOG=N ==========
balazs.kilvady@imgtec.com changed reviewers: + jkummerow@chromium.org, mstarzinger@chromium.org - bradnelson@google.com
balazs.kilvady@imgtec.com changed reviewers: + bradnelson@google.com - jkummerow@chromium.org, mstarzinger@chromium.org
https://codereview.chromium.org/1778673004/diff/1/test/mjsunit/mjsunit.status File test/mjsunit/mjsunit.status (right): https://codereview.chromium.org/1778673004/diff/1/test/mjsunit/mjsunit.status... test/mjsunit/mjsunit.status:312: 'wasm/asm-wasm': [PASS, ['arch in [arm, arm64, mips, mipsel, mips64, mips64el]', SKIP]], On 2016/03/09 18:11:17, bradn wrote: > I'm confused, doesn't this disable asm-wasm for mips? It does, that is our purpose. On MIPS trunc.d.w converts a double value which is higher then MAX_INT to MAX_INT so asm-wasm test fails. ( Failing part is: function TestConvertF64FromUnsigned() { "use asm"; function caller() { var a = 0xffffffff; if ((+(a>>>0)) > 0.0) { if((+a) < 0.0) { return 26; } } return 0; } return {caller:caller}; } The test fails on a target board and passes on simulator because of different conversion, I think. The static_cast<>() of the following code produces different results on Intel and on MIPS: int val = static_cast<int>(expr->raw_value()->AsNumber()); PrintF("double val: %f, int converted: 0x%x\n", expr->raw_value()->AsNumber(), val); The 'good' result of Intel (sim) is: double val: 4294967295.000000, int converted: 0x80000000 the 'bad' result on MIPS board: double val: 4294967295.000000, int converted: 0x7fffffff Do you think we should do some workaround solution for MIPS (and perhaps for ARM, too) or we can just skip the test like ARM guys did?
Disable is fine, it might be good long term to split these out. The commit message is confusing, perhaps change to: "Disable asm-wasm mjsunit tests on mips" ? Otherwise LGTM
Description was changed from ========== MIPS: Enable asm-wasm mjsunit tests (skip on arm and mips). Port 879d254d54eb1d92721c421aebb9a43faff9eb8a Original commit message: Landing this again, excluding arm as it fail and arm64 out of caution that other wasm arm64 tests are also disabled. BUG= https://code.google.com/p/v8/issues/detail?id=4203 TEST=mjsunit/asm-wasm LOG=N ========== to ========== MIPS: Disable asm-wasm mjsunit tests. Port 879d254d54eb1d92721c421aebb9a43faff9eb8a Original commit message: Landing this again, excluding arm as it fail and arm64 out of caution that other wasm arm64 tests are also disabled. BUG= https://code.google.com/p/v8/issues/detail?id=4203 TEST=mjsunit/asm-wasm LOG=N ==========
The CQ bit was checked by balazs.kilvady@imgtec.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1778673004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1778673004/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_presubmit on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/12480)
balazs.kilvady@imgtec.com changed reviewers: + danno@chromium.org, jkummerow@chromium.org, machenbach@google.com
PTAL, It seems from the presubmit log that we need an lgtm from an owner:
Missing LGTM from an OWNER for these files:
test/mjsunit/mjsunit.status
machenbach@chromium.org changed reviewers: + machenbach@chromium.org
lgtm
The CQ bit was checked by balazs.kilvady@imgtec.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1778673004/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1778673004/1
Message was sent while issue was closed.
Description was changed from ========== MIPS: Disable asm-wasm mjsunit tests. Port 879d254d54eb1d92721c421aebb9a43faff9eb8a Original commit message: Landing this again, excluding arm as it fail and arm64 out of caution that other wasm arm64 tests are also disabled. BUG= https://code.google.com/p/v8/issues/detail?id=4203 TEST=mjsunit/asm-wasm LOG=N ========== to ========== MIPS: Disable asm-wasm mjsunit tests. Port 879d254d54eb1d92721c421aebb9a43faff9eb8a Original commit message: Landing this again, excluding arm as it fail and arm64 out of caution that other wasm arm64 tests are also disabled. BUG= https://code.google.com/p/v8/issues/detail?id=4203 TEST=mjsunit/asm-wasm LOG=N ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== MIPS: Disable asm-wasm mjsunit tests. Port 879d254d54eb1d92721c421aebb9a43faff9eb8a Original commit message: Landing this again, excluding arm as it fail and arm64 out of caution that other wasm arm64 tests are also disabled. BUG= https://code.google.com/p/v8/issues/detail?id=4203 TEST=mjsunit/asm-wasm LOG=N ========== to ========== MIPS: Disable asm-wasm mjsunit tests. Port 879d254d54eb1d92721c421aebb9a43faff9eb8a Original commit message: Landing this again, excluding arm as it fail and arm64 out of caution that other wasm arm64 tests are also disabled. BUG= https://code.google.com/p/v8/issues/detail?id=4203 TEST=mjsunit/asm-wasm LOG=N Committed: https://crrev.com/7b9dca76b8bca685ef95473b795e83227bbf0768 Cr-Commit-Position: refs/heads/master@{#34860} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/7b9dca76b8bca685ef95473b795e83227bbf0768 Cr-Commit-Position: refs/heads/master@{#34860} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
