Skip to content

Commit

Permalink
[vm/compiler] Make flow graph construction independent of --target_un…
Browse files Browse the repository at this point in the history
…known_cpu

Move optimized handling of Double.truncateToDouble, floorToDouble,
ceilToDouble, floor and ceil from flow graph construction to
the canonicalization pass.

This is needed in order to maintain stable flow graphs between
app-jit snapshot training (which is performed with --target_unknown_cpu)
and running from app-jit snapshot without --target_unknown_cpu.

TEST=ci (vm-appjit-*)
Fixes #56018

Change-Id: I7e3e4ce917984f52db239506ea3de0c55af00567
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/372140
Reviewed-by: Slava Egorov <[email protected]>
Commit-Queue: Alexander Markov <[email protected]>
  • Loading branch information
alexmarkov authored and Commit Queue committed Jun 18, 2024
1 parent d430e7a commit 7bd8b31
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 42 deletions.
7 changes: 0 additions & 7 deletions runtime/bin/main_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1321,13 +1321,6 @@ void main(int argc, char** argv) {
vm_options.AddArgument("--link_natives_lazily");
#endif
}
if (app_snapshot != nullptr && app_snapshot->IsJIT()) {
// App-jit snapshot was created with this flag, it also has to be added
// when running the snapshot when it is depended upon by the graph builder
// (see e.g. FlowGraphBuilder::IsRecognizedMethodForFlowGraph).
// TODO(dartbug.com/56018) Better fix leaving less performance on the table.
vm_options.AddArgument("--target-unknown-cpu");
}

// If we need to write an app-jit snapshot or a depfile, then add an exit
// hook that writes the snapshot and/or depfile as appropriate.
Expand Down
61 changes: 61 additions & 0 deletions runtime/vm/compiler/backend/il.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7260,6 +7260,67 @@ const RuntimeEntry& InvokeMathCFunctionInstr::TargetFunction() const {
return kLibcPowRuntimeEntry;
}

Definition* InvokeMathCFunctionInstr::Canonicalize(FlowGraph* flow_graph) {
if (!CompilerState::Current().is_aot() &&
TargetCPUFeatures::double_truncate_round_supported()) {
Token::Kind op_kind = Token::kILLEGAL;
switch (recognized_kind_) {
case MethodRecognizer::kDoubleTruncateToDouble:
op_kind = Token::kTRUNCATE;
break;
case MethodRecognizer::kDoubleFloorToDouble:
op_kind = Token::kFLOOR;
break;
case MethodRecognizer::kDoubleCeilToDouble:
op_kind = Token::kCEILING;
break;
default:
return this;
}
auto* instr = new UnaryDoubleOpInstr(
op_kind, new Value(InputAt(0)->definition()), GetDeoptId(),
Instruction::kNotSpeculative, kUnboxedDouble);
flow_graph->InsertBefore(this, instr, env(), FlowGraph::kValue);
return instr;
}

return this;
}

bool DoubleToIntegerInstr::SupportsFloorAndCeil() {
#if defined(TARGET_ARCH_X64)
return CompilerState::Current().is_aot() || FLAG_target_unknown_cpu;
#elif defined(TARGET_ARCH_ARM64) || defined(TARGET_ARCH_RISCV32) || \
defined(TARGET_ARCH_RISCV64)
return true;
#else
return false;
#endif
}

Definition* DoubleToIntegerInstr::Canonicalize(FlowGraph* flow_graph) {
if (SupportsFloorAndCeil() &&
(recognized_kind() == MethodRecognizer::kDoubleToInteger)) {
if (auto* arg = value()->definition()->AsInvokeMathCFunction()) {
switch (arg->recognized_kind()) {
case MethodRecognizer::kDoubleFloorToDouble:
// x.floorToDouble().toInt() => x.floor()
recognized_kind_ = MethodRecognizer::kDoubleFloorToInt;
value()->BindTo(arg->InputAt(0)->definition());
break;
case MethodRecognizer::kDoubleCeilToDouble:
// x.ceilToDouble().toInt() => x.ceil()
recognized_kind_ = MethodRecognizer::kDoubleCeilToInt;
value()->BindTo(arg->InputAt(0)->definition());
break;
default:
break;
}
}
}
return this;
}

TruncDivModInstr::TruncDivModInstr(Value* lhs, Value* rhs, intptr_t deopt_id)
: TemplateDefinition(deopt_id) {
SetInputAt(0, lhs);
Expand Down
8 changes: 7 additions & 1 deletion runtime/vm/compiler/backend/il.h
Original file line number Diff line number Diff line change
Expand Up @@ -10084,14 +10084,18 @@ class DoubleToIntegerInstr : public TemplateDefinition<1, Throws, Pure> {
return other.AsDoubleToInteger()->recognized_kind() == recognized_kind();
}

#define FIELD_LIST(F) F(const MethodRecognizer::Kind, recognized_kind_)
virtual Definition* Canonicalize(FlowGraph* flow_graph);

#define FIELD_LIST(F) F(MethodRecognizer::Kind, recognized_kind_)

DECLARE_INSTRUCTION_SERIALIZABLE_FIELDS(DoubleToIntegerInstr,
TemplateDefinition,
FIELD_LIST)
#undef FIELD_LIST

private:
static bool SupportsFloorAndCeil();

DISALLOW_COPY_AND_ASSIGN(DoubleToIntegerInstr);
};

Expand Down Expand Up @@ -10284,6 +10288,8 @@ class InvokeMathCFunctionInstr : public VariadicDefinition {
return other_invoke->recognized_kind() == recognized_kind();
}

virtual Definition* Canonicalize(FlowGraph* flow_graph);

virtual bool MayThrow() const { return false; }

static constexpr intptr_t kSavedSpTempIndex = 0;
Expand Down
1 change: 1 addition & 0 deletions runtime/vm/compiler/backend/il_arm.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5578,6 +5578,7 @@ LocationSummary* DoubleToIntegerInstr::MakeLocationSummary(Zone* zone,
}

void DoubleToIntegerInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
ASSERT(recognized_kind() == MethodRecognizer::kDoubleToInteger);
const Register result = locs()->out(0).reg();
const DRegister value_double = EvenDRegisterOf(locs()->in(0).fpu_reg());

Expand Down
36 changes: 2 additions & 34 deletions runtime/vm/compiler/frontend/kernel_to_il.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1140,16 +1140,6 @@ bool FlowGraphBuilder::IsRecognizedMethodForFlowGraph(
case MethodRecognizer::kMathLog:
case MethodRecognizer::kMathSqrt:
return true;
case MethodRecognizer::kDoubleCeilToInt:
case MethodRecognizer::kDoubleFloorToInt:
#if defined(TARGET_ARCH_X64)
return CompilerState::Current().is_aot() || FLAG_target_unknown_cpu;
#elif defined(TARGET_ARCH_ARM64) || defined(TARGET_ARCH_RISCV32) || \
defined(TARGET_ARCH_RISCV64)
return true;
#else
return false;
#endif
default:
return false;
}
Expand Down Expand Up @@ -1814,9 +1804,7 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod(
body += LoadIndexed(kIntPtrCid);
body += Box(kUnboxedIntPtr);
} break;
case MethodRecognizer::kDoubleToInteger:
case MethodRecognizer::kDoubleCeilToInt:
case MethodRecognizer::kDoubleFloorToInt: {
case MethodRecognizer::kDoubleToInteger: {
body += LoadLocal(parsed_function_->RawParameterVariable(0));
body += DoubleToInteger(kind);
} break;
Expand All @@ -1839,27 +1827,7 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfRecognizedMethod(
for (intptr_t i = 0, n = function.NumParameters(); i < n; ++i) {
body += LoadLocal(parsed_function_->RawParameterVariable(i));
}
if (!CompilerState::Current().is_aot() &&
TargetCPUFeatures::double_truncate_round_supported() &&
((kind == MethodRecognizer::kDoubleTruncateToDouble) ||
(kind == MethodRecognizer::kDoubleFloorToDouble) ||
(kind == MethodRecognizer::kDoubleCeilToDouble))) {
switch (kind) {
case MethodRecognizer::kDoubleTruncateToDouble:
body += UnaryDoubleOp(Token::kTRUNCATE);
break;
case MethodRecognizer::kDoubleFloorToDouble:
body += UnaryDoubleOp(Token::kFLOOR);
break;
case MethodRecognizer::kDoubleCeilToDouble:
body += UnaryDoubleOp(Token::kCEILING);
break;
default:
UNREACHABLE();
}
} else {
body += InvokeMathCFunction(kind, function.NumParameters());
}
body += InvokeMathCFunction(kind, function.NumParameters());
} break;
case MethodRecognizer::kMathSqrt: {
body += LoadLocal(parsed_function_->RawParameterVariable(0));
Expand Down

0 comments on commit 7bd8b31

Please sign in to comment.