Commit c84bcd74 authored by Seth Brenith's avatar Seth Brenith Committed by V8 LUCI CQ

Revert "Shorten generated code for binary-search switches"

This reverts commit 00a757fa.

Reason for revert: Caused perf regressions, https://crbug.com/1280236

Original change's description:
> Shorten generated code for binary-search switches
>
> On some branches of the search tree for a binary-search switch, the
> input value is sufficiently constrained that we could unconditionally
> jump to the last possible case rather than checking for value equality.
> This shortens some builtins by a few instructions and might speed things
> up, though I expect the effect to be small.
>
> Change-Id: I2313f26976e6d3c182f03bd927b338c8175b3af3
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3335437
> Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
> Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
> Cr-Commit-Position: refs/heads/main@{#78376}

Bug: chromium:1280236
Change-Id: I88d9ff64641b85d48198b7012df2eeb9441913b5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3343234
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: 's avatarNico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#78397}
parent 0c681483
...@@ -3625,9 +3625,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr, ...@@ -3625,9 +3625,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr,
void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) { void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) {
ArmOperandConverter i(this, instr); ArmOperandConverter i(this, instr);
Register input = i.InputRegister(0); Register input = i.InputRegister(0);
std::vector<std::pair<int32_t, RpoNumber>> cases; std::vector<std::pair<int32_t, Label*>> cases;
for (size_t index = 2; index < instr->InputCount(); index += 2) { for (size_t index = 2; index < instr->InputCount(); index += 2) {
cases.push_back({i.InputInt32(index + 0), i.InputRpo(index + 1)}); cases.push_back({i.InputInt32(index + 0), GetLabel(i.InputRpo(index + 1))});
} }
AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(), AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(),
cases.data() + cases.size()); cases.data() + cases.size());
......
...@@ -2960,9 +2960,9 @@ void CodeGenerator::AssembleArchSelect(Instruction* instr, ...@@ -2960,9 +2960,9 @@ void CodeGenerator::AssembleArchSelect(Instruction* instr,
void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) { void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) {
Arm64OperandConverter i(this, instr); Arm64OperandConverter i(this, instr);
Register input = i.InputRegister32(0); Register input = i.InputRegister32(0);
std::vector<std::pair<int32_t, RpoNumber>> cases; std::vector<std::pair<int32_t, Label*>> cases;
for (size_t index = 2; index < instr->InputCount(); index += 2) { for (size_t index = 2; index < instr->InputCount(); index += 2) {
cases.push_back({i.InputInt32(index + 0), i.InputRpo(index + 1)}); cases.push_back({i.InputInt32(index + 0), GetLabel(i.InputRpo(index + 1))});
} }
AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(), AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(),
cases.data() + cases.size()); cases.data() + cases.size());
......
...@@ -491,23 +491,11 @@ void CodeGenerator::AssembleCode() { ...@@ -491,23 +491,11 @@ void CodeGenerator::AssembleCode() {
} }
void CodeGenerator::AssembleArchBinarySearchSwitchRange( void CodeGenerator::AssembleArchBinarySearchSwitchRange(
Register input, RpoNumber def_block, std::pair<int32_t, RpoNumber>* begin, Register input, RpoNumber def_block, std::pair<int32_t, Label*>* begin,
std::pair<int32_t, RpoNumber>* end, int32_t min_possible_value, std::pair<int32_t, Label*>* end) {
int32_t max_possible_value) {
if (end - begin < kBinarySearchSwitchMinimalCases) { if (end - begin < kBinarySearchSwitchMinimalCases) {
while (begin != end) { while (begin != end) {
if (min_possible_value == begin->first) { tasm()->JumpIfEqual(input, begin->first, begin->second);
if (max_possible_value == begin->first) {
// Only one value is possible, so we can jump to it unconditionally.
DCHECK_EQ(begin + 1, end);
def_block = begin->second;
break;
}
// After emitting the branch for this case, the minimum possible value
// is increased.
++min_possible_value;
}
tasm()->JumpIfEqual(input, begin->first, GetLabel(begin->second));
++begin; ++begin;
} }
AssembleArchJumpRegardlessOfAssemblyOrder(def_block); AssembleArchJumpRegardlessOfAssemblyOrder(def_block);
...@@ -516,11 +504,9 @@ void CodeGenerator::AssembleArchBinarySearchSwitchRange( ...@@ -516,11 +504,9 @@ void CodeGenerator::AssembleArchBinarySearchSwitchRange(
auto middle = begin + (end - begin) / 2; auto middle = begin + (end - begin) / 2;
Label less_label; Label less_label;
tasm()->JumpIfLessThan(input, middle->first, &less_label); tasm()->JumpIfLessThan(input, middle->first, &less_label);
AssembleArchBinarySearchSwitchRange(input, def_block, middle, end, AssembleArchBinarySearchSwitchRange(input, def_block, middle, end);
middle->first, max_possible_value);
tasm()->bind(&less_label); tasm()->bind(&less_label);
AssembleArchBinarySearchSwitchRange(input, def_block, begin, middle, AssembleArchBinarySearchSwitchRange(input, def_block, begin, middle);
min_possible_value, middle->first - 1);
} }
void CodeGenerator::AssembleArchJump(RpoNumber target) { void CodeGenerator::AssembleArchJump(RpoNumber target) {
......
...@@ -257,11 +257,9 @@ class V8_EXPORT_PRIVATE CodeGenerator final : public GapResolver::Assembler { ...@@ -257,11 +257,9 @@ class V8_EXPORT_PRIVATE CodeGenerator final : public GapResolver::Assembler {
#if V8_ENABLE_WEBASSEMBLY #if V8_ENABLE_WEBASSEMBLY
void AssembleArchTrap(Instruction* instr, FlagsCondition condition); void AssembleArchTrap(Instruction* instr, FlagsCondition condition);
#endif // V8_ENABLE_WEBASSEMBLY #endif // V8_ENABLE_WEBASSEMBLY
void AssembleArchBinarySearchSwitchRange( void AssembleArchBinarySearchSwitchRange(Register input, RpoNumber def_block,
Register input, RpoNumber def_block, std::pair<int32_t, RpoNumber>* begin, std::pair<int32_t, Label*>* begin,
std::pair<int32_t, RpoNumber>* end, std::pair<int32_t, Label*>* end);
int32_t min_possible_value = std::numeric_limits<int32_t>::min(),
int32_t max_possible_value = std::numeric_limits<int32_t>::max());
void AssembleArchBinarySearchSwitch(Instruction* instr); void AssembleArchBinarySearchSwitch(Instruction* instr);
void AssembleArchTableSwitch(Instruction* instr); void AssembleArchTableSwitch(Instruction* instr);
......
...@@ -3841,9 +3841,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr, ...@@ -3841,9 +3841,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr,
void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) { void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) {
IA32OperandConverter i(this, instr); IA32OperandConverter i(this, instr);
Register input = i.InputRegister(0); Register input = i.InputRegister(0);
std::vector<std::pair<int32_t, RpoNumber>> cases; std::vector<std::pair<int32_t, Label*>> cases;
for (size_t index = 2; index < instr->InputCount(); index += 2) { for (size_t index = 2; index < instr->InputCount(); index += 2) {
cases.push_back({i.InputInt32(index + 0), i.InputRpo(index + 1)}); cases.push_back({i.InputInt32(index + 0), GetLabel(i.InputRpo(index + 1))});
} }
AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(), AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(),
cases.data() + cases.size()); cases.data() + cases.size());
......
...@@ -2152,9 +2152,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr, ...@@ -2152,9 +2152,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr,
void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) { void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) {
Loong64OperandConverter i(this, instr); Loong64OperandConverter i(this, instr);
Register input = i.InputRegister(0); Register input = i.InputRegister(0);
std::vector<std::pair<int32_t, RpoNumber>> cases; std::vector<std::pair<int32_t, Label*>> cases;
for (size_t index = 2; index < instr->InputCount(); index += 2) { for (size_t index = 2; index < instr->InputCount(); index += 2) {
cases.push_back({i.InputInt32(index + 0), i.InputRpo(index + 1)}); cases.push_back({i.InputInt32(index + 0), GetLabel(i.InputRpo(index + 1))});
} }
AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(), AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(),
cases.data() + cases.size()); cases.data() + cases.size());
......
...@@ -3906,9 +3906,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr, ...@@ -3906,9 +3906,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr,
void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) { void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) {
MipsOperandConverter i(this, instr); MipsOperandConverter i(this, instr);
Register input = i.InputRegister(0); Register input = i.InputRegister(0);
std::vector<std::pair<int32_t, RpoNumber>> cases; std::vector<std::pair<int32_t, Label*>> cases;
for (size_t index = 2; index < instr->InputCount(); index += 2) { for (size_t index = 2; index < instr->InputCount(); index += 2) {
cases.push_back({i.InputInt32(index + 0), i.InputRpo(index + 1)}); cases.push_back({i.InputInt32(index + 0), GetLabel(i.InputRpo(index + 1))});
} }
AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(), AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(),
cases.data() + cases.size()); cases.data() + cases.size());
......
...@@ -4110,9 +4110,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr, ...@@ -4110,9 +4110,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr,
void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) { void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) {
MipsOperandConverter i(this, instr); MipsOperandConverter i(this, instr);
Register input = i.InputRegister(0); Register input = i.InputRegister(0);
std::vector<std::pair<int32_t, RpoNumber>> cases; std::vector<std::pair<int32_t, Label*>> cases;
for (size_t index = 2; index < instr->InputCount(); index += 2) { for (size_t index = 2; index < instr->InputCount(); index += 2) {
cases.push_back({i.InputInt32(index + 0), i.InputRpo(index + 1)}); cases.push_back({i.InputInt32(index + 0), GetLabel(i.InputRpo(index + 1))});
} }
AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(), AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(),
cases.data() + cases.size()); cases.data() + cases.size());
......
...@@ -3926,9 +3926,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr, ...@@ -3926,9 +3926,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr,
void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) { void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) {
PPCOperandConverter i(this, instr); PPCOperandConverter i(this, instr);
Register input = i.InputRegister(0); Register input = i.InputRegister(0);
std::vector<std::pair<int32_t, RpoNumber>> cases; std::vector<std::pair<int32_t, Label*>> cases;
for (size_t index = 2; index < instr->InputCount(); index += 2) { for (size_t index = 2; index < instr->InputCount(); index += 2) {
cases.push_back({i.InputInt32(index + 0), i.InputRpo(index + 1)}); cases.push_back({i.InputInt32(index + 0), GetLabel(i.InputRpo(index + 1))});
} }
AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(), AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(),
cases.data() + cases.size()); cases.data() + cases.size());
......
...@@ -3764,9 +3764,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr, ...@@ -3764,9 +3764,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr,
void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) { void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) {
RiscvOperandConverter i(this, instr); RiscvOperandConverter i(this, instr);
Register input = i.InputRegister(0); Register input = i.InputRegister(0);
std::vector<std::pair<int32_t, RpoNumber>> cases; std::vector<std::pair<int32_t, Label*>> cases;
for (size_t index = 2; index < instr->InputCount(); index += 2) { for (size_t index = 2; index < instr->InputCount(); index += 2) {
cases.push_back({i.InputInt32(index + 0), i.InputRpo(index + 1)}); cases.push_back({i.InputInt32(index + 0), GetLabel(i.InputRpo(index + 1))});
} }
AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(), AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(),
cases.data() + cases.size()); cases.data() + cases.size());
......
...@@ -3793,9 +3793,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr, ...@@ -3793,9 +3793,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr,
void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) { void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) {
S390OperandConverter i(this, instr); S390OperandConverter i(this, instr);
Register input = i.InputRegister(0); Register input = i.InputRegister(0);
std::vector<std::pair<int32_t, RpoNumber>> cases; std::vector<std::pair<int32_t, Label*>> cases;
for (size_t index = 2; index < instr->InputCount(); index += 2) { for (size_t index = 2; index < instr->InputCount(); index += 2) {
cases.push_back({i.InputInt32(index + 0), i.InputRpo(index + 1)}); cases.push_back({i.InputInt32(index + 0), GetLabel(i.InputRpo(index + 1))});
} }
AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(), AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(),
cases.data() + cases.size()); cases.data() + cases.size());
......
...@@ -4478,9 +4478,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr, ...@@ -4478,9 +4478,9 @@ void CodeGenerator::AssembleArchBoolean(Instruction* instr,
void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) { void CodeGenerator::AssembleArchBinarySearchSwitch(Instruction* instr) {
X64OperandConverter i(this, instr); X64OperandConverter i(this, instr);
Register input = i.InputRegister(0); Register input = i.InputRegister(0);
std::vector<std::pair<int32_t, RpoNumber>> cases; std::vector<std::pair<int32_t, Label*>> cases;
for (size_t index = 2; index < instr->InputCount(); index += 2) { for (size_t index = 2; index < instr->InputCount(); index += 2) {
cases.push_back({i.InputInt32(index + 0), i.InputRpo(index + 1)}); cases.push_back({i.InputInt32(index + 0), GetLabel(i.InputRpo(index + 1))});
} }
AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(), AssembleArchBinarySearchSwitchRange(input, i.InputRpo(1), cases.data(),
cases.data() + cases.size()); cases.data() + cases.size());
......
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