Commit a1fe961c authored by Tobias Tebbi's avatar Tobias Tebbi Committed by Commit Bot

Revert "[turbofan] Consistently use String feedback for JSAdd."

This reverts commit d504203e.

Reason for revert: b/74469145

Original change's description:
> [turbofan] Consistently use String feedback for JSAdd.
> 
> Currently we didn't always consistently use the String feedback on
> JSAdd, but only if JSTypedLowering would already figure out statically
> that one of the inputs is already a String. That leads to some odd
> performance cliffs, as highlighted in the referenced bug.
> 
> This CL fixes the JSTypedLowering::ReduceJSAdd to always bake in the
> String feedback. This improves the relevant performance tests from the
> bug from
> 
>   console.timeEnd: Runtime join3, 967.512000
>   console.timeEnd: Runtime join, 1004.599000
>   console.timeEnd: Runtime join3, 1124.764000
>   console.timeEnd: Runtime join, 966.164000
>   console.timeEnd: Runtime join3, 1145.296000
>   console.timeEnd: Runtime join, 966.176000
>   console.timeEnd: Runtime join3, 1145.272000
>   console.timeEnd: Runtime join, 931.266000
> 
> to
> 
>   console.timeEnd: Runtime join3, 903.050000
>   console.timeEnd: Runtime join, 856.509000
>   console.timeEnd: Runtime join3, 945.144000
>   console.timeEnd: Runtime join, 840.038000
>   console.timeEnd: Runtime join3, 927.965000
>   console.timeEnd: Runtime join, 841.263000
>   console.timeEnd: Runtime join3, 929.342000
>   console.timeEnd: Runtime join, 858.143000
> 
> which corresponds to an 8-18% improvement.
> 
> Bug: v8:7415
> Change-Id: I62e008298e4ee0864885b37817c91d055acf2a09
> Reviewed-on: https://chromium-review.googlesource.com/936643
> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#51551}

TBR=jarin@chromium.org,bmeurer@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: v8:7415
Change-Id: I106a314bcd4187abdad6dc11306226d0c28ef524
Reviewed-on: https://chromium-review.googlesource.com/963522Reviewed-by: 's avatarTobias Tebbi <tebbi@chromium.org>
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51949}
parent 3032a9ff
...@@ -504,13 +504,6 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) { ...@@ -504,13 +504,6 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
r.ConvertInputsToNumber(); r.ConvertInputsToNumber();
return r.ChangeToPureOperator(simplified()->NumberAdd(), Type::Number()); return r.ChangeToPureOperator(simplified()->NumberAdd(), Type::Number());
} }
if (BinaryOperationHintOf(node->op()) == BinaryOperationHint::kString) {
// Always bake in String feedback into the graph.
// TODO(bmeurer): Consider adding a SpeculativeStringAdd operator,
// and use that in JSTypeHintLowering instead of looking at the
// binary operation feedback here.
r.CheckInputsToString();
}
if (r.OneInputIs(Type::String())) { if (r.OneInputIs(Type::String())) {
// We know that (at least) one input is already a String, // We know that (at least) one input is already a String,
// so try to strength-reduce the non-String input. // so try to strength-reduce the non-String input.
...@@ -546,7 +539,7 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) { ...@@ -546,7 +539,7 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
return ReduceCreateConsString(node); return ReduceCreateConsString(node);
} }
// Eliminate useless concatenation of empty string. // Eliminate useless concatenation of empty string.
if (r.BothInputsAre(Type::String())) { if (BinaryOperationHintOf(node->op()) == BinaryOperationHint::kString) {
Node* effect = NodeProperties::GetEffectInput(node); Node* effect = NodeProperties::GetEffectInput(node);
Node* control = NodeProperties::GetControlInput(node); Node* control = NodeProperties::GetControlInput(node);
if (r.LeftInputIs(empty_string_type_)) { if (r.LeftInputIs(empty_string_type_)) {
...@@ -589,8 +582,6 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) { ...@@ -589,8 +582,6 @@ Reduction JSTypedLowering::ReduceJSAdd(Node* node) {
NodeProperties::ChangeOp(node, common()->Call(call_descriptor)); NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
return Changed(node); return Changed(node);
} }
// We never get here when we had String feedback.
DCHECK_NE(BinaryOperationHint::kString, BinaryOperationHintOf(node->op()));
return NoChange(); return NoChange();
} }
......
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