Commit 02fc37d3 authored by Darius M's avatar Darius M Committed by V8 LUCI CQ

[compiler] Better code generation for branches on binops

Commit 0719ace6 improves the code
generated for comparisons by avoiding the materalization of the
comparison bit.

Now, this commit aims at doing this same improvement for binary
operations. Since binary operations set the ZF flag, there is no
reason to insert a "== 0" comparison.

Note that this commit might increase register pressure, which might
actually reduce performance. It's hard to anticipate, so we'll land
it, and revert it if it's actually bad for performance.


Bug: v8:12484
Change-Id: I963f0c4afdd59b35b4bac468e47d987836433163
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3545165Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
Commit-Queue: Darius Mercadier <dmercadier@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79611}
parent bcf43eb7
...@@ -18,15 +18,29 @@ namespace { ...@@ -18,15 +18,29 @@ namespace {
bool IsBranch(Node* node) { return node->opcode() == IrOpcode::kBranch; } bool IsBranch(Node* node) { return node->opcode() == IrOpcode::kBranch; }
bool CanDuplicate(Node* node) { bool CanDuplicate(Node* node) {
// We only allow duplication of comparisons. Duplicating other nodes makes // We only allow duplication of comparisons and "cheap" binary operations
// little sense, because a "== 0" need to be inserted in branches anyways // (cheap = not multiplication or division). The idea is that those
// (except for some binops, but duplicating binops might increase the live // instructions set the ZF flag, and thus do not require a "== 0" to be added
// range of the operands, thus increasing register pressure). // before the branch. Duplicating other nodes, on the other hand, makes little
// sense, because a "== 0" would need to be inserted in branches anyways.
switch (node->opcode()) { switch (node->opcode()) {
#define BRANCH_CASE(op) \ #define BRANCH_CASE(op) \
case IrOpcode::k##op: \ case IrOpcode::k##op: \
break; break;
MACHINE_COMPARE_BINOP_LIST(BRANCH_CASE) MACHINE_COMPARE_BINOP_LIST(BRANCH_CASE)
case IrOpcode::kInt32Add:
case IrOpcode::kInt32Sub:
case IrOpcode::kWord32And:
case IrOpcode::kWord32Or:
case IrOpcode::kInt64Add:
case IrOpcode::kInt64Sub:
case IrOpcode::kWord64And:
case IrOpcode::kWord64Or:
case IrOpcode::kWord32Shl:
case IrOpcode::kWord32Shr:
case IrOpcode::kWord64Shl:
case IrOpcode::kWord64Shr:
break;
default: default:
return false; return false;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment