mirror of
https://github.com/dart-lang/sdk
synced 2024-09-15 21:40:07 +00:00
Reland "[vm/compiler] Remove unnecessary type check from optimized switches"
This is a reland of commit 43ce5487c1
In addition to removing type check, switch range checks are adjusted
in order to avoid call to 'int.operator<=', which can be removed by
tree shaker in AOT/product mode, causing NoSuchMethodError at runtime.
Original change's description:
> [vm/compiler] Remove unnecessary type check from optimized switches
>
> Switch optimization now relies on static type of the tested value,
> so it is safe to omit the type check (with sound null safety) or
> reduce it to a null check (without sound null safety).
>
> TEST=co19/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t01
> Fixes https://github.com/dart-lang/sdk/issues/52422
>
> Change-Id: Ic93f4f212bee9ed3bfe5035f3c8d7535274c2f63
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304102
> Commit-Queue: Alexander Markov <alexmarkov@google.com>
> Reviewed-by: Alexander Aprelev <aam@google.com>
TEST=co19/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_enum_A01_t01
TEST=language/async/switch_test
Change-Id: I70bf480bf376f946d66caa504120c8f85093ea8d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/304322
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
This commit is contained in:
parent
34a21f5691
commit
0bff5652ba
|
@ -5072,30 +5072,23 @@ Fragment StreamingFlowGraphBuilder::BuildOptimizedSwitchPrelude(
|
|||
SwitchHelper* helper,
|
||||
JoinEntryInstr* join) {
|
||||
const TokenPosition pos = helper->position();
|
||||
|
||||
// We need to check that the switch variable is of the correct type.
|
||||
// If it is not, we go to [join] which is either the default case or
|
||||
// the exit of the switch statement.
|
||||
|
||||
TargetEntryInstr* then_entry;
|
||||
TargetEntryInstr* otherwise_entry;
|
||||
|
||||
const AbstractType& expression_type = helper->expression_type();
|
||||
ASSERT(dart::SimpleInstanceOfType(expression_type));
|
||||
|
||||
Fragment instructions;
|
||||
|
||||
if (!IG->null_safety()) {
|
||||
// Without sound null safety we need to check that the switch variable is
|
||||
// not null. If it is null, we go to [join] which is either the default
|
||||
// case or the exit of the switch statement.
|
||||
TargetEntryInstr* null_entry;
|
||||
TargetEntryInstr* non_null_entry;
|
||||
|
||||
instructions += LoadLocal(scopes()->switch_variable);
|
||||
instructions += Constant(expression_type);
|
||||
instructions += InstanceCall(
|
||||
pos, Library::PrivateCoreLibName(Symbols::_simpleInstanceOf()),
|
||||
Token::kIS, /*argument_count=*/2,
|
||||
/*checked_argument_count=*/2);
|
||||
instructions += BranchIfTrue(&then_entry, &otherwise_entry, /*negate=*/false);
|
||||
instructions += BranchIfNull(&null_entry, &non_null_entry);
|
||||
|
||||
Fragment otherwise_instructions(otherwise_entry);
|
||||
otherwise_instructions += Goto(join);
|
||||
Fragment null_instructions(null_entry);
|
||||
null_instructions += Goto(join);
|
||||
|
||||
instructions = Fragment(instructions.entry, then_entry);
|
||||
instructions = Fragment(instructions.entry, non_null_entry);
|
||||
}
|
||||
|
||||
if (helper->is_enum_switch()) {
|
||||
// For an enum switch, we need to load the enum index from the switch
|
||||
|
@ -5224,10 +5217,7 @@ Fragment StreamingFlowGraphBuilder::BuildBinarySearchSwitch(
|
|||
branch_instructions += LoadLocal(scopes()->switch_variable);
|
||||
branch_instructions += Constant(middle_expression.integer());
|
||||
branch_instructions +=
|
||||
InstanceCall(middle_expression.position(),
|
||||
Symbols::LessEqualOperator(), Token::kLTE,
|
||||
/*argument_count=*/2,
|
||||
/*checked_argument_count=*/2);
|
||||
B->IntRelationalOp(middle_expression.position(), Token::kLTE);
|
||||
branch_instructions +=
|
||||
BranchIfTrue(&then_entry, &otherwise_entry, /*negate=*/false);
|
||||
|
||||
|
@ -5242,10 +5232,7 @@ Fragment StreamingFlowGraphBuilder::BuildBinarySearchSwitch(
|
|||
upper_branch_instructions += LoadLocal(scopes()->switch_variable);
|
||||
upper_branch_instructions += Constant(next_expression.integer());
|
||||
upper_branch_instructions +=
|
||||
InstanceCall(next_expression.position(),
|
||||
Symbols::GreaterEqualOperator(), Token::kGTE,
|
||||
/*argument_count=*/2,
|
||||
/*checked_argument_count=*/2);
|
||||
B->IntRelationalOp(next_expression.position(), Token::kGTE);
|
||||
upper_branch_instructions +=
|
||||
BranchIfTrue(&then_entry, &otherwise_entry, /*negate=*/false);
|
||||
|
||||
|
@ -5260,6 +5247,10 @@ Fragment StreamingFlowGraphBuilder::BuildBinarySearchSwitch(
|
|||
stack.Add(
|
||||
SwitchRange::Branch(range.min(), middle, lower_branch_instructions));
|
||||
}
|
||||
|
||||
if (current_instructions.is_empty()) {
|
||||
current_instructions = branch_instructions;
|
||||
}
|
||||
}
|
||||
|
||||
return Fragment(current_instructions.entry, join_instructions.current);
|
||||
|
@ -5302,9 +5293,7 @@ Fragment StreamingFlowGraphBuilder::BuildJumpTableSwitch(SwitchHelper* helper) {
|
|||
if (helper->RequiresLowerBoundCheck()) {
|
||||
current_instructions += LoadLocal(scopes()->switch_variable);
|
||||
current_instructions += Constant(expression_min);
|
||||
current_instructions += InstanceCall(pos, Symbols::GreaterEqualOperator(),
|
||||
Token::kGTE, /*argument_count=*/2,
|
||||
/*checked_argument_count=*/2);
|
||||
current_instructions += B->IntRelationalOp(pos, Token::kGTE);
|
||||
current_instructions += BranchIfTrue(&then_entry, &otherwise_entry,
|
||||
/*negate=*/false);
|
||||
Fragment otherwise_instructions(otherwise_entry);
|
||||
|
@ -5316,9 +5305,7 @@ Fragment StreamingFlowGraphBuilder::BuildJumpTableSwitch(SwitchHelper* helper) {
|
|||
if (helper->RequiresUpperBoundCheck()) {
|
||||
current_instructions += LoadLocal(scopes()->switch_variable);
|
||||
current_instructions += Constant(expression_max);
|
||||
current_instructions += InstanceCall(pos, Symbols::LessEqualOperator(),
|
||||
Token::kLTE, /*argument_count=*/2,
|
||||
/*checked_argument_count=*/2);
|
||||
current_instructions += B->IntRelationalOp(pos, Token::kLTE);
|
||||
current_instructions += BranchIfTrue(&then_entry, &otherwise_entry,
|
||||
/*negate=*/false);
|
||||
Fragment otherwise_instructions(otherwise_entry);
|
||||
|
|
|
@ -4287,6 +4287,39 @@ Fragment FlowGraphBuilder::IntToBool() {
|
|||
return body;
|
||||
}
|
||||
|
||||
Fragment FlowGraphBuilder::IntRelationalOp(TokenPosition position,
|
||||
Token::Kind kind) {
|
||||
if (CompilerState::Current().is_aot()) {
|
||||
Value* right = Pop();
|
||||
Value* left = Pop();
|
||||
RelationalOpInstr* instr = new (Z) RelationalOpInstr(
|
||||
InstructionSource(position), kind, left, right, kMintCid,
|
||||
GetNextDeoptId(), Instruction::SpeculativeMode::kNotSpeculative);
|
||||
Push(instr);
|
||||
return Fragment(instr);
|
||||
}
|
||||
const String* name = nullptr;
|
||||
switch (kind) {
|
||||
case Token::kLT:
|
||||
name = &Symbols::LAngleBracket();
|
||||
break;
|
||||
case Token::kGT:
|
||||
name = &Symbols::RAngleBracket();
|
||||
break;
|
||||
case Token::kLTE:
|
||||
name = &Symbols::LessEqualOperator();
|
||||
break;
|
||||
case Token::kGTE:
|
||||
name = &Symbols::GreaterEqualOperator();
|
||||
break;
|
||||
default:
|
||||
UNREACHABLE();
|
||||
}
|
||||
return InstanceCall(
|
||||
position, *name, kind, /*type_args_len=*/0, /*argument_count=*/2,
|
||||
/*argument_names=*/Array::null_array(), /*checked_argument_count=*/2);
|
||||
}
|
||||
|
||||
Fragment FlowGraphBuilder::NativeReturn(
|
||||
const compiler::ffi::CallbackMarshaller& marshaller) {
|
||||
auto* instr = new (Z)
|
||||
|
|
|
@ -298,6 +298,9 @@ class FlowGraphBuilder : public BaseFlowGraphBuilder {
|
|||
// Converts 0 to false and the rest to true.
|
||||
Fragment IntToBool();
|
||||
|
||||
// Compares arbitrary integers.
|
||||
Fragment IntRelationalOp(TokenPosition position, Token::Kind kind);
|
||||
|
||||
// Creates an ffi.Pointer holding a given address.
|
||||
Fragment FfiPointerFromAddress();
|
||||
|
||||
|
|
Loading…
Reference in a new issue