Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2652893013:
[ARM] Macro-ize SIMD visitors in InstructionSelector. (Closed)
Description[ARM] Macro-ize SIMD visitors in InstructionSelector.
- Uses macros for splat, extract lane, replace lane, unary
ops and binary ops.
- Renames ARM SIMD instruction codes to match machine
operator names.
LOG=N
BUG=v8:4124
Review-Url: https://codereview.chromium.org/2652893013
Cr-Commit-Position: refs/heads/master@{#42799}
Committed: https://chromium.googlesource.com/v8/v8/+/c0b1bcba2f33565201eaf3148b10dbe5d4c9231a
Patch Set 1 #
Total comments: 4
Patch Set 2 : Review comments. #Patch Set 3 : Int32MulHigh has 3 operands. #
Messages
Total messages: 32 (20 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...
bbudge@chromium.org changed reviewers: + martyn.capewell@arm.com
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by bbudge@chromium.org
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
Try jobs failed on following builders: v8_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/33624)
bbudge@chromium.org changed reviewers: + titzer@chromium.org
+Ben for OWNERS
https://codereview.chromium.org/2652893013/diff/1/src/compiler/arm/instructio... File src/compiler/arm/instruction-selector-arm.cc (right): https://codereview.chromium.org/2652893013/diff/1/src/compiler/arm/instructio... src/compiler/arm/instruction-selector-arm.cc:1471: void InstructionSelector::VisitFloat32Mul(Node* node) { Can you do the same for these instructions too? https://codereview.chromium.org/2652893013/diff/1/src/compiler/arm/instructio... src/compiler/arm/instruction-selector-arm.cc:2346: void InstructionSelector::VisitCreate##Type(Node* node) { \ Can you make these macros all call VisitRR or VisitRRR? (that saves on the generated C++ code)
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...
https://codereview.chromium.org/2652893013/diff/1/src/compiler/arm/instructio... File src/compiler/arm/instruction-selector-arm.cc (right): https://codereview.chromium.org/2652893013/diff/1/src/compiler/arm/instructio... src/compiler/arm/instruction-selector-arm.cc:1471: void InstructionSelector::VisitFloat32Mul(Node* node) { On 2017/01/30 21:01:25, titzer wrote: > Can you do the same for these instructions too? Done. https://codereview.chromium.org/2652893013/diff/1/src/compiler/arm/instructio... src/compiler/arm/instruction-selector-arm.cc:2346: void InstructionSelector::VisitCreate##Type(Node* node) { \ On 2017/01/30 21:01:25, titzer wrote: > Can you make these macros all call VisitRR or VisitRRR? (that saves on the > generated C++ code) Done.
Nice! LGTM
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux_arm_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng/builds/...) v8_linux_arm_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel_ng_trigger...)
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 martyn.capewell@arm.com, titzer@chromium.org Link to the patchset: https://codereview.chromium.org/2652893013/#ps40001 (title: "Int32MulHigh has 3 operands.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/01/31 01:32:23, commit-bot: I haz the power wrote: > CQ is trying da patch. Follow status at > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... Would you like to do the same for the other platforms, Bill?
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1485826337758090,
"parent_rev": "93c65bbffb5f1bcf5a101969f47d7c23656fb10d", "commit_rev":
"c0b1bcba2f33565201eaf3148b10dbe5d4c9231a"}
Message was sent while issue was closed.
Description was changed from ========== [ARM] Macro-ize SIMD visitors in InstructionSelector. - Uses macros for splat, extract lane, replace lane, unary ops and binary ops. - Renames ARM SIMD instruction codes to match machine operator names. LOG=N BUG=v8:4124 ========== to ========== [ARM] Macro-ize SIMD visitors in InstructionSelector. - Uses macros for splat, extract lane, replace lane, unary ops and binary ops. - Renames ARM SIMD instruction codes to match machine operator names. LOG=N BUG=v8:4124 Review-Url: https://codereview.chromium.org/2652893013 Cr-Commit-Position: refs/heads/master@{#42799} Committed: https://chromium.googlesource.com/v8/v8/+/c0b1bcba2f33565201eaf3148b10dbe5d4c... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/v8/v8/+/c0b1bcba2f33565201eaf3148b10dbe5d4c...
Message was sent while issue was closed.
On 2017/01/31 01:33:35, titzer wrote: > On 2017/01/31 01:32:23, commit-bot: I haz the power wrote: > > CQ is trying da patch. Follow status at > > > > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... > > Would you like to do the same for the other platforms, Bill? I'll have to go off my OCD meds, but OK, will do. Maybe x64 and arm64 to start. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
