Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2400513002:
[Turbofan] Allow FP operands and vregs in InstructionSequenceTest. (Closed)
|
Created:
4 years, 2 months ago by bbudge Modified:
4 years, 2 months ago Reviewers:
Mircea Trofin CC:
v8-reviews_googlegroups.com Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[Turbofan] Allow FP operands and vregs in InstructionSequenceTest.
- Adds an optional representation field to VReg and TestOperand structs.
- Adds a simple FP allocation test to register-allocator-unittest.cc.
- Adds some simple FP tests to move-optimizer-unittest.cc.
LOG=N
BUG=v8:4124
Committed: https://crrev.com/5c4298a0ae825e699997eba21e1ebd52943e28b3
Cr-Commit-Position: refs/heads/master@{#40117}
Patch Set 1 #Patch Set 2 : Restore implicit conversion. #Patch Set 3 : Fix move optimizer test. #Patch Set 4 : MoveOptimizer tests shouldn't pass UnallocatedOperands. #Patch Set 5 : Distinguish tests that require register allocation. #Patch Set 6 : Rebase. #
Total comments: 4
Patch Set 7 : Review comments. #Patch Set 8 : Rebase. #Patch Set 9 : Remove extra tests, will add back later. #
Messages
Total messages: 42 (36 generated)
The CQ bit was checked by bbudge@chromium.org 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: Try jobs failed on following builders: v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...) v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...) v8_mac_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel_ng/builds/10220)
bbudge@chromium.org changed reviewers: + mtrofin@chromium.org
The CQ bit was checked by bbudge@chromium.org 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 bbudge@chromium.org 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 bbudge@chromium.org 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 bbudge@chromium.org 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: Try jobs failed on following builders: v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/15716)
The CQ bit was checked by bbudge@chromium.org 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.
lgtm, 2 nits https://codereview.chromium.org/2400513002/diff/100001/test/unittests/compile... File test/unittests/compiler/instruction-sequence-unittest.h (right): https://codereview.chromium.org/2400513002/diff/100001/test/unittests/compile... test/unittests/compiler/instruction-sequence-unittest.h:23: static const MachineRepresentation kFloat32 = MachineRepresentation::kFloat32; why do we need this re-assignment? https://codereview.chromium.org/2400513002/diff/100001/test/unittests/compile... test/unittests/compiler/instruction-sequence-unittest.h:212: virtual bool DoesRegisterAllocation() { return true; } can you mark this const, please?
The CQ bit was checked by bbudge@chromium.org 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: Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
https://codereview.chromium.org/2400513002/diff/100001/test/unittests/compile... File test/unittests/compiler/instruction-sequence-unittest.h (right): https://codereview.chromium.org/2400513002/diff/100001/test/unittests/compile... test/unittests/compiler/instruction-sequence-unittest.h:23: static const MachineRepresentation kFloat32 = MachineRepresentation::kFloat32; On 2016/10/08 16:38:56, Mircea Trofin wrote: > why do we need this re-assignment? I'll use kFloat32 and kSimd128 when I add aliasing tests. But I'll delete them for now. https://codereview.chromium.org/2400513002/diff/100001/test/unittests/compile... test/unittests/compiler/instruction-sequence-unittest.h:212: virtual bool DoesRegisterAllocation() { return true; } On 2016/10/08 16:38:56, Mircea Trofin wrote: > can you mark this const, please? Done.
The CQ bit was checked by bbudge@chromium.org 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 bbudge@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mtrofin@chromium.org Link to the patchset: https://codereview.chromium.org/2400513002/#ps160001 (title: "Remove extra tests, will add back later.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #9 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== [Turbofan] Allow FP operands and vregs in InstructionSequenceTest. - Adds an optional representation field to VReg and TestOperand structs. - Adds a simple FP allocation test to register-allocator-unittest.cc. - Adds some simple FP tests to move-optimizer-unittest.cc. LOG=N BUG=v8:4124 ========== to ========== [Turbofan] Allow FP operands and vregs in InstructionSequenceTest. - Adds an optional representation field to VReg and TestOperand structs. - Adds a simple FP allocation test to register-allocator-unittest.cc. - Adds some simple FP tests to move-optimizer-unittest.cc. LOG=N BUG=v8:4124 Committed: https://crrev.com/5c4298a0ae825e699997eba21e1ebd52943e28b3 Cr-Commit-Position: refs/heads/master@{#40117} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/5c4298a0ae825e699997eba21e1ebd52943e28b3 Cr-Commit-Position: refs/heads/master@{#40117} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
