From 7d8ca951ec60217d6f1d45ae898fef521e2d04f4 Mon Sep 17 00:00:00 2001 From: Toon Verwaest Date: Fri, 27 Jan 2023 17:22:15 +0100 Subject: [PATCH] [maglev] Don't check smi overflow after int32 unary/binop This means TurboFan might not see what Maglev did, and it might make different decisions, but if we deopt we'll learn in Ignition anyway and won't make the same mistake later. At the same time this avoids a lot of unnecessary operations that impact tight loops. Bug: v8:7700 Change-Id: I6fada2ed0218b0b97fc8c9d9ba10fb2218cd71d4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4200631 Reviewed-by: Leszek Swirski Auto-Submit: Toon Verwaest Commit-Queue: Leszek Swirski Cr-Commit-Position: refs/heads/main@{#85585} --- src/maglev/maglev-graph-builder.cc | 23 ++----------------- test/mjsunit/maglev/add-smi.js | 5 ++-- .../mjsunit/maglev/shift-right-logical-smi.js | 6 ++--- test/mjsunit/maglev/shift-right-logical.js | 6 ++--- 4 files changed, 10 insertions(+), 30 deletions(-) diff --git a/src/maglev/maglev-graph-builder.cc b/src/maglev/maglev-graph-builder.cc index 8d1c5cda00..9e625f4028 100644 --- a/src/maglev/maglev-graph-builder.cc +++ b/src/maglev/maglev-graph-builder.cc @@ -494,13 +494,7 @@ void MaglevGraphBuilder::BuildInt32UnaryOperationNode() { ValueNode* value = input_is_truncated ? GetAccumulatorTruncatedInt32() : GetAccumulatorInt32(); using OpNodeT = Int32NodeFor; - OpNodeT* result = AddNewNode({value}); - NodeInfo* node_info = known_node_aspects().GetOrCreateInfoFor(result); - node_info->type = NodeType::kSmi; - static_assert(OpNodeT::kProperties.value_representation() == - ValueRepresentation::kInt32); - node_info->tagged_alternative = AddNewNode({result}); - SetAccumulator(result); + SetAccumulator(AddNewNode({value})); } void MaglevGraphBuilder::BuildTruncatingInt32BitwiseNotForNumber() { @@ -567,20 +561,7 @@ void MaglevGraphBuilder::BuildInt32BinaryOperationNode() { } using OpNodeT = Int32NodeFor; - OpNodeT* result = AddNewNode({left, right}); - NodeInfo* node_info = known_node_aspects().GetOrCreateInfoFor(result); - node_info->type = NodeType::kSmi; - if constexpr (OpNodeT::kProperties.value_representation() == - ValueRepresentation::kInt32) { - // For Int32, the check is the same as a tag operation, so we may as well - // keep the tagged result as the tagged alternative. - node_info->tagged_alternative = AddNewNode({result}); - } else { - static_assert(OpNodeT::kProperties.value_representation() == - ValueRepresentation::kUint32); - AddNewNode({result}); - } - SetAccumulator(result); + SetAccumulator(AddNewNode({left, right})); } template diff --git a/test/mjsunit/maglev/add-smi.js b/test/mjsunit/maglev/add-smi.js index c59e97a070..e2802c8324 100644 --- a/test/mjsunit/maglev/add-smi.js +++ b/test/mjsunit/maglev/add-smi.js @@ -17,9 +17,8 @@ assertEquals(3, add(1, 2)); assertTrue(isMaglevved(add)); - // We should deopt here in SmiUntag. assertEquals(0x40000000, add(1, 0x3FFFFFFF)); - assertFalse(isMaglevved(add)); + assertTrue(isMaglevved(add)); })(); // Checks when we deopt due to tagging. @@ -35,7 +34,7 @@ assertEquals(3, add(1, 2)); assertTrue(isMaglevved(add)); - // We should deopt here in SmiTag. + // We should deopt here in Int32Add. assertEquals(3.2, add(1.2, 2)); assertFalse(isMaglevved(add)); })(); diff --git a/test/mjsunit/maglev/shift-right-logical-smi.js b/test/mjsunit/maglev/shift-right-logical-smi.js index b24121b08d..6077d3c4ac 100644 --- a/test/mjsunit/maglev/shift-right-logical-smi.js +++ b/test/mjsunit/maglev/shift-right-logical-smi.js @@ -39,10 +39,10 @@ function shrl_test_expect_deopt(lhs, rhs, expected_result) { } shrl_test(8, 2, 2); -shrl_test_expect_deopt(-1, 1, 2147483647); +shrl_test(-1, 1, 2147483647); shrl_test(-8, 2, 1073741822); -shrl_test_expect_deopt(-8, 0, 4294967288); -shrl_test_expect_deopt(-892396978, 0, 3402570318); +shrl_test(-8, 0, 4294967288); +shrl_test(-892396978, 0, 3402570318); shrl_test(8, 10, 0); shrl_test(8, 33, 4); shrl_test_expect_deopt(0xFFFFFFFF, 0x3FFFFFFF, 1); diff --git a/test/mjsunit/maglev/shift-right-logical.js b/test/mjsunit/maglev/shift-right-logical.js index 1b30e21ec8..164fd0d057 100644 --- a/test/mjsunit/maglev/shift-right-logical.js +++ b/test/mjsunit/maglev/shift-right-logical.js @@ -35,10 +35,10 @@ function shrl_test_expect_deopt(lhs, rhs, expected_result) { } shrl_test(8, 2, 2); -shrl_test_expect_deopt(-1, 1, 2147483647); +shrl_test(-1, 1, 2147483647); shrl_test(-8, 2, 1073741822); -shrl_test_expect_deopt(-8, 0, 4294967288); -shrl_test_expect_deopt(-892396978, 0, 3402570318); +shrl_test(-8, 0, 4294967288); +shrl_test(-892396978, 0, 3402570318); shrl_test(8, 10, 0); shrl_test(8, 33, 4); shrl_test_expect_deopt(0xFFFFFFFF, 0x3FFFFFFF, 1);