[vm/compiler] Improve flow sensitivity of CP

Before running CP find all branches of form `Branch(v eq C)`,
where `eq` is one of `===`, `==`, `!==` or `!=` and `C` is
a constant and insert a redefinition of `v` in the true
successor which captures the information that `v` is equal
to `C` guaranteed by the branch.

SCCP algorithm can then use this information when propagating
constants. After SCCP all redefinitions inserted are removed
again.

We don't actually replace `v` with `C` in the dominated code
because this might lead to issues later, for example
it might result in redundant phis `phi(C, v)` (`C` flowing
from a predecessor where `v == C`) which are complicated
to eliminate again.

TEST=vm/dart/comparison_canonicalization_il_test

Cq-Include-Trybots: luci.dart.try:vm-aot-linux-release-x64-try,vm-aot-linux-debug-simarm_x64-try,vm-aot-linux-debug-x64-try,vm-aot-android-release-arm64c-try
Change-Id: I388dddb97b5e35f09d9904000c585864f27400f6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/324980
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This commit is contained in:
Vyacheslav Egorov 2023-09-12 22:39:25 +00:00 committed by Commit Queue
parent 16cc32f0ec
commit 5c320a108a
7 changed files with 509 additions and 48 deletions

View file

@ -54,19 +54,27 @@ class FlowGraph {
}
void dump() {
String formatOne(Map<String, dynamic> instr) {
final inputs = instr['i']?.map((v) => 'v$v').join(', ') ?? '';
final successors = instr['s'] != null ? ' goto ${instr['s']}' : '';
final attrs = descriptors[instr['o']]
?.attributeIndex
.entries
.map((e) => '${e.key}: ${instr['d'][e.value]}')
.join(',');
final condition = instr['cc'] != null ? formatOne(instr['cc']) : '';
final attrsWrapped = attrs != null ? '[$attrs]' : '';
final inputsWrapped =
condition != '' ? ' if $condition then ' : '($inputs)';
return '${instr['o']}$attrsWrapped$inputsWrapped$successors';
}
for (var block in blocks) {
print('B${block['b']}[${block['o']}]');
for (var instr in [...?block['d'], ...?block['is']]) {
final v = instr['v'] ?? -1;
final prefix = v != -1 ? 'v$v <- ' : '';
final inputs = instr['i']?.map((v) => 'v$v').join(', ') ?? '';
final attrs = descriptors[instr['o']]
?.attributeIndex
.entries
.map((e) => '${e.key}: ${instr['d'][e.value]}')
.join(',');
final attrsWrapped = attrs != null ? '[$attrs]' : '';
print(' ${prefix}${instr['o']}$attrsWrapped($inputs)');
print(' ${prefix}${formatOne(instr)}');
}
}
}

View file

@ -0,0 +1,313 @@
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// Given
//
// Branch if StrictCompare(v === Constant(C)) then B1 else B2
//
// constant propagation should treat all uses of `v` dominated by
// B1 as if they were equal to C.
//
// The same applies to the negated case, where `v` is equal to `C`
// in the false successor. The same applies to EqualityCompare() when applied
// to integers.
//
// Note that we don't want to actually eagerly replace `v` with `C` because
// it might complicate subsequent optimizations by introducing redundant phis.
import 'package:vm/testing/il_matchers.dart';
class A {
final int v;
const A(this.v);
}
@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int strictCompareValueEqConstant(A value) {
if (value == const A(0)) {
return value.v;
} else {
return 42;
}
}
void matchIL$strictCompareValueEqConstant(FlowGraph graph) {
graph.match([
match.block('Graph', [
'A(0)' << match.Constant(value: "Instance of A"),
'int 0' << match.UnboxedConstant(value: 0),
'int 42' << match.UnboxedConstant(value: 42),
]),
match.block('Function', [
'value' << match.Parameter(index: 0),
match.Branch(match.StrictCompare('value', 'A(0)', kind: '==='),
ifTrue: 'B1', ifFalse: 'B2'),
]),
'B1' <<
match.block('Target', [
match.Return('int 0'),
]),
'B2' <<
match.block('Target', [
match.Return('int 42'),
]),
]);
}
@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int strictCompareConstantEqValue(A value) {
if (const A(0) == value) {
return value.v;
} else {
return 42;
}
}
void matchIL$strictCompareConstantEqValue(FlowGraph graph) {
graph.match([
match.block('Graph', [
'A(0)' << match.Constant(value: "Instance of A"),
'int 0' << match.UnboxedConstant(value: 0),
'int 42' << match.UnboxedConstant(value: 42),
]),
match.block('Function', [
'value' << match.Parameter(index: 0),
match.Branch(match.StrictCompare('A(0)', 'value', kind: '==='),
ifTrue: 'B1', ifFalse: 'B2'),
]),
'B1' <<
match.block('Target', [
match.Return('int 0'),
]),
'B2' <<
match.block('Target', [
match.Return('int 42'),
]),
]);
}
@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int strictCompareValueNeConstant(A value) {
if (value != const A(0)) {
return 42;
} else {
return value.v;
}
}
void matchIL$strictCompareValueNeConstant(FlowGraph graph) {
graph.match([
match.block('Graph', [
'A(0)' << match.Constant(value: "Instance of A"),
'int 0' << match.UnboxedConstant(value: 0),
'int 42' << match.UnboxedConstant(value: 42),
]),
match.block('Function', [
'value' << match.Parameter(index: 0),
match.Branch(match.StrictCompare('value', 'A(0)', kind: '!=='),
ifTrue: 'B1', ifFalse: 'B2'),
]),
'B1' <<
match.block('Target', [
match.Return('int 42'),
]),
'B2' <<
match.block('Target', [
match.Return('int 0'),
]),
]);
}
@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int strictCompareConstantNeValue(A value) {
if (const A(0) != value) {
return 42;
} else {
return value.v;
}
}
void matchIL$strictCompareConstantNeValue(FlowGraph graph) {
graph.match([
match.block('Graph', [
'A(0)' << match.Constant(value: "Instance of A"),
'int 0' << match.UnboxedConstant(value: 0),
'int 42' << match.UnboxedConstant(value: 42),
]),
match.block('Function', [
'value' << match.Parameter(index: 0),
match.Branch(match.StrictCompare('A(0)', 'value', kind: '!=='),
ifTrue: 'B1', ifFalse: 'B2'),
]),
'B1' <<
match.block('Target', [
match.Return('int 42'),
]),
'B2' <<
match.block('Target', [
match.Return('int 0'),
]),
]);
}
@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
bool strictCompareBoolEqTrue(bool value) {
// Note: expect false to be propagated into the false successor as well.
if (value == true) {
return !value;
} else {
return !value;
}
}
void matchIL$strictCompareBoolEqTrue(FlowGraph graph) {
graph.match([
match.block('Graph', [
'true' << match.Constant(value: true),
'false' << match.Constant(value: false),
]),
match.block('Function', [
'value' << match.Parameter(index: 0),
match.Branch(match.StrictCompare('value', 'true', kind: '==='),
ifTrue: 'B1', ifFalse: 'B2'),
]),
'B1' <<
match.block('Target', [
match.Return('false'),
]),
'B2' <<
match.block('Target', [
match.Return('true'),
]),
]);
}
@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
bool strictCompareBoolNeTrue(bool value) {
// Note: expect false to be propagated into the true successor as well.
if (value != true) {
return !value;
} else {
return !value;
}
}
void matchIL$strictCompareBoolNeTrue(FlowGraph graph) {
graph.match([
match.block('Graph', [
'true' << match.Constant(value: true),
'false' << match.Constant(value: false),
]),
match.block('Function', [
'value' << match.Parameter(index: 0),
match.Branch(match.StrictCompare('value', 'true', kind: '!=='),
ifTrue: 'B1', ifFalse: 'B2'),
]),
'B1' <<
match.block('Target', [
match.Return('true'),
]),
'B2' <<
match.block('Target', [
match.Return('false'),
]),
]);
}
@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int equalityCompareValueEqConstant(int value) {
if (value == 0) {
return value + 1;
} else {
return 42;
}
}
void matchIL$equalityCompareValueEqConstant(FlowGraph graph) {
graph.match([
match.block('Graph', [
'int 0' << match.UnboxedConstant(value: 0),
'int 1' << match.UnboxedConstant(value: 1),
'int 42' << match.UnboxedConstant(value: 42),
]),
match.block('Function', [
'value' << match.Parameter(index: 0),
match.Branch(match.EqualityCompare('value', 'int 0', kind: '=='),
ifTrue: 'B1', ifFalse: 'B2'),
]),
'B1' <<
match.block('Target', [
match.Return('int 1'),
]),
'B2' <<
match.block('Target', [
match.Return('int 42'),
]),
]);
}
@pragma('vm:never-inline')
@pragma('vm:testing:print-flow-graph')
int foldingOfRepeatedComparison(int value) {
final v = value >= 1;
if (v) {
if (v) {
return 1;
} else {
return 24;
}
} else {
return 42;
}
}
void matchIL$foldingOfRepeatedComparison(FlowGraph graph) {
graph.match([
match.block('Graph', [
'int 1' << match.UnboxedConstant(value: 1),
'int 42' << match.UnboxedConstant(value: 42),
]),
match.block('Function', [
'value' << match.Parameter(index: 0),
match.Branch(match.RelationalOp('value', 'int 1', kind: '>='),
ifTrue: 'B1', ifFalse: 'B2'),
]),
'B1' <<
match.block('Target', [
match.Return('int 1'),
]),
'B2' <<
match.block('Target', [
match.Return('int 42'),
]),
]);
}
void main(List<String> args) {
for (var v in [
A(0),
A(1),
A(42),
A(int.parse(args.isEmpty ? "24" : args[0])),
]) {
print(strictCompareValueEqConstant(v));
print(strictCompareConstantEqValue(v));
print(strictCompareValueNeConstant(v));
print(strictCompareConstantNeValue(v));
print(equalityCompareValueEqConstant(v.v));
print(foldingOfRepeatedComparison(v.v));
}
for (var v in [true, false]) {
print(strictCompareBoolEqTrue(v));
print(strictCompareBoolNeTrue(v));
}
}

View file

@ -384,6 +384,10 @@ void ConstantPropagator::VisitPhi(PhiInstr* instr) {
}
void ConstantPropagator::VisitRedefinition(RedefinitionInstr* instr) {
if (instr->inserted_by_constant_propagation()) {
return;
}
const Object& value = instr->value()->definition()->constant_value();
if (IsConstant(value)) {
SetValue(instr, value);
@ -1519,7 +1523,78 @@ void ConstantPropagator::VisitUnaryUint32Op(UnaryUint32OpInstr* instr) {
SetValue(instr, non_constant_);
}
// Insert redefinition for |original| definition which conveys information
// that |original| is equal to |constant_value| in the dominated code.
static RedefinitionInstr* InsertRedefinition(FlowGraph* graph,
BlockEntryInstr* dom,
Definition* original,
const Object& constant_value) {
auto redef = new RedefinitionInstr(new Value(original),
/*inserted_by_constant_propagation=*/true);
graph->InsertAfter(dom, redef, nullptr, FlowGraph::kValue);
graph->RenameDominatedUses(original, redef, redef);
if (redef->input_use_list() == nullptr) {
// There are no dominated uses, so the newly added Redefinition is useless.
redef->RemoveFromGraph();
return nullptr;
}
redef->constant_value() = constant_value.ptr();
return redef;
}
// Find all Branch(v eq constant) (eq being one of ==, !=, === or !==) in the
// graph and redefine |v| in the true successor to record information about
// it being equal to the constant.
//
// We don't actually _replace_ |v| with |constant| in the dominated code
// because it might complicate subsequent optimizations (e.g. lead to
// redundant phis).
void ConstantPropagator::InsertRedefinitionsAfterEqualityComparisons() {
for (auto block : graph_->reverse_postorder()) {
if (auto branch = block->last_instruction()->AsBranch()) {
auto comparison = branch->comparison();
if (comparison->IsStrictCompare() ||
(comparison->IsEqualityCompare() &&
comparison->operation_cid() != kDoubleCid)) {
Value* value;
ConstantInstr* constant_defn;
if (comparison->IsComparisonWithConstant(&value, &constant_defn) &&
!value->BindsToConstant()) {
const Object& constant_value = constant_defn->value();
// Found comparison with constant. Introduce Redefinition().
ASSERT(comparison->kind() == Token::kNE_STRICT ||
comparison->kind() == Token::kNE ||
comparison->kind() == Token::kEQ_STRICT ||
comparison->kind() == Token::kEQ);
const bool negated = (comparison->kind() == Token::kNE_STRICT ||
comparison->kind() == Token::kNE);
const auto true_successor =
negated ? branch->false_successor() : branch->true_successor();
InsertRedefinition(graph_, true_successor, value->definition(),
constant_value);
// When comparing two boolean values we can also apply renaming
// to the false successor because we know that only true and false
// are possible values.
if (constant_value.IsBool() && value->Type()->IsBool()) {
const auto false_successor =
negated ? branch->true_successor() : branch->false_successor();
InsertRedefinition(graph_, false_successor, value->definition(),
Bool::Get(!Bool::Cast(constant_value).value()));
}
}
}
}
}
}
void ConstantPropagator::Analyze() {
InsertRedefinitionsAfterEqualityComparisons();
GraphEntryInstr* entry = graph_->graph_entry();
reachable_->Add(entry->preorder_number());
block_worklist_.Add(entry);
@ -1759,10 +1834,21 @@ void ConstantPropagator::Transform() {
}
bool ConstantPropagator::TransformDefinition(Definition* defn) {
if (defn == nullptr) {
return false;
}
if (auto redef = defn->AsRedefinition()) {
if (redef->inserted_by_constant_propagation()) {
redef->ReplaceUsesWith(redef->value()->definition());
return true;
}
}
// Replace constant-valued instructions without observable side
// effects. Do this for smis and old objects only to avoid having to
// copy other objects into the heap's old generation.
if ((defn != nullptr) && IsConstant(defn->constant_value()) &&
if (IsConstant(defn->constant_value()) &&
(defn->constant_value().IsSmi() || defn->constant_value().IsOld()) &&
!defn->IsConstant() && !defn->IsStoreIndexed() && !defn->IsStoreField() &&
!defn->IsStoreStaticField()) {

View file

@ -39,6 +39,7 @@ class ConstantPropagator : public FlowGraphVisitor {
static ObjectPtr Unknown() { return Object::unknown_constant().ptr(); }
private:
void InsertRedefinitionsAfterEqualityComparisons();
void Analyze();
void Transform();
// Tries to replace uses of [defn] with a constant, returns true if

View file

@ -1127,6 +1127,14 @@ bool Value::BindsToConstant() const {
return definition()->OriginalDefinition()->IsConstant();
}
bool Value::BindsToConstant(ConstantInstr** constant_defn) const {
if (auto constant = definition()->OriginalDefinition()->AsConstant()) {
*constant_defn = constant;
return true;
}
return false;
}
// Returns true if the value represents constant null.
bool Value::BindsToConstantNull() const {
ConstantInstr* constant = definition()->OriginalDefinition()->AsConstant();
@ -3380,51 +3388,39 @@ static Definition* CanonicalizeStrictCompare(StrictCompareInstr* compare,
}
}
*negated = false;
PassiveObject& constant = PassiveObject::Handle();
ConstantInstr* constant_defn = nullptr;
Value* other = nullptr;
if (compare->right()->BindsToConstant()) {
constant = compare->right()->BoundConstant().ptr();
other = compare->left();
} else if (compare->left()->BindsToConstant()) {
constant = compare->left()->BoundConstant().ptr();
other = compare->right();
} else {
if (!compare->IsComparisonWithConstant(&other, &constant_defn)) {
return compare;
}
const Object& constant = constant_defn->value();
const bool can_merge = is_branch || (other->Type()->ToCid() == kBoolCid);
Definition* other_defn = other->definition();
Token::Kind kind = compare->kind();
// Handle e === true.
if ((kind == Token::kEQ_STRICT) && (constant.ptr() == Bool::True().ptr()) &&
can_merge) {
if (!constant.IsBool() || !can_merge) {
return compare;
}
const bool constant_value = Bool::Cast(constant).value();
// Handle `e === true` and `e !== false`: these cases don't require
// negation and allow direct merge.
if ((kind == Token::kEQ_STRICT) == constant_value) {
return other_defn;
}
// Handle e !== false.
if ((kind == Token::kNE_STRICT) && (constant.ptr() == Bool::False().ptr()) &&
can_merge) {
return other_defn;
}
// Handle e !== true.
if ((kind == Token::kNE_STRICT) && (constant.ptr() == Bool::True().ptr()) &&
other_defn->IsComparison() && can_merge &&
other_defn->HasOnlyUse(other)) {
ComparisonInstr* comp = other_defn->AsComparison();
if (!IsFpCompare(comp)) {
*negated = true;
return other_defn;
}
}
// Handle e === false.
if ((kind == Token::kEQ_STRICT) && (constant.ptr() == Bool::False().ptr()) &&
other_defn->IsComparison() && can_merge &&
other_defn->HasOnlyUse(other)) {
ComparisonInstr* comp = other_defn->AsComparison();
if (!IsFpCompare(comp)) {
// We now have `e !== true` or `e === false`: these cases require
// negation.
if (auto comp = other_defn->AsComparison()) {
if (other_defn->HasOnlyUse(other) && !IsFpCompare(comp)) {
*negated = true;
return other_defn;
}
}
return compare;
}
@ -3470,11 +3466,10 @@ static bool RecognizeTestPattern(Value* left, Value* right, bool* negate) {
Instruction* BranchInstr::Canonicalize(FlowGraph* flow_graph) {
Zone* zone = flow_graph->zone();
// Only handle strict-compares.
if (comparison()->IsStrictCompare()) {
bool negated = false;
Definition* replacement = CanonicalizeStrictCompare(
comparison()->AsStrictCompare(), &negated, /* is_branch = */ true);
comparison()->AsStrictCompare(), &negated, /*is_branch=*/true);
if (replacement == comparison()) {
return this;
}
@ -3508,8 +3503,12 @@ Instruction* BranchInstr::Canonicalize(FlowGraph* flow_graph) {
comp->ClearSSATempIndex();
comp->ClearTempIndex();
}
} else if (comparison()->IsEqualityCompare() &&
comparison()->operation_cid() == kSmiCid) {
return this;
}
if (comparison()->IsEqualityCompare() &&
comparison()->operation_cid() == kSmiCid) {
BinarySmiOpInstr* bit_and = nullptr;
bool negate = false;
if (RecognizeTestPattern(comparison()->left(), comparison()->right(),
@ -3543,7 +3542,7 @@ Definition* StrictCompareInstr::Canonicalize(FlowGraph* flow_graph) {
bool negated = false;
Definition* replacement = CanonicalizeStrictCompare(this, &negated,
/* is_branch = */ false);
/*is_branch=*/false);
if (negated && replacement->IsComparison()) {
ASSERT(replacement != this);
replacement->AsComparison()->NegateComparison();

View file

@ -158,6 +158,7 @@ class Value : public ZoneAllocated {
// Return true if the value represents a constant.
bool BindsToConstant() const;
bool BindsToConstant(ConstantInstr** constant_defn) const;
// Return true if the value represents the constant null.
bool BindsToConstantNull() const;
@ -665,10 +666,18 @@ using serializable_type_t =
#define DECLARE_ATTRIBUTES(...) \
auto GetAttributes() const { return std::make_tuple(__VA_ARGS__); } \
static auto GetAttributeNames() { return std::make_tuple(#__VA_ARGS__); }
#define DECLARE_ATTRIBUTES_NAMED(names, values) \
auto GetAttributes() const { \
return std::make_tuple values; \
} \
static auto GetAttributeNames() { \
return std::make_tuple names; \
}
#else
#define PRINT_TO_SUPPORT
#define PRINT_OPERANDS_TO_SUPPORT
#define DECLARE_ATTRIBUTES(...)
#define DECLARE_ATTRIBUTES_NAMED(names, values)
#endif // defined(INCLUDE_IL_PRINTER)
// Together with CidRange, this represents a mapping from a range of class-ids
@ -3681,6 +3690,21 @@ class ComparisonInstr : public Definition {
(operation_cid() == other_comparison->operation_cid());
}
// Detects comparison with a constant and returns constant and the other
// operand.
bool IsComparisonWithConstant(Value** other_operand,
ConstantInstr** constant_operand) {
if (right()->BindsToConstant(constant_operand)) {
*other_operand = left();
return true;
} else if (left()->BindsToConstant(constant_operand)) {
*other_operand = right();
return true;
} else {
return false;
}
}
DECLARE_ABSTRACT_INSTRUCTION(Comparison)
#define FIELD_LIST(F) \
@ -3875,7 +3899,10 @@ class DeoptimizeInstr : public TemplateInstruction<0, NoThrow, Pure> {
class RedefinitionInstr : public TemplateDefinition<1, NoThrow> {
public:
explicit RedefinitionInstr(Value* value) : constrained_type_(nullptr) {
explicit RedefinitionInstr(Value* value,
bool inserted_by_constant_propagation = false)
: constrained_type_(nullptr),
inserted_by_constant_propagation_(inserted_by_constant_propagation) {
SetInputAt(0, value);
}
@ -3891,6 +3918,10 @@ class RedefinitionInstr : public TemplateDefinition<1, NoThrow> {
void set_constrained_type(CompileType* type) { constrained_type_ = type; }
CompileType* constrained_type() const { return constrained_type_; }
bool inserted_by_constant_propagation() const {
return inserted_by_constant_propagation_;
}
virtual bool ComputeCanDeoptimize() const { return false; }
virtual bool HasUnknownSideEffects() const { return false; }
@ -3898,7 +3929,9 @@ class RedefinitionInstr : public TemplateDefinition<1, NoThrow> {
PRINT_OPERANDS_TO_SUPPORT
#define FIELD_LIST(F) F(CompileType*, constrained_type_)
#define FIELD_LIST(F) \
F(CompileType*, constrained_type_) \
F(bool, inserted_by_constant_propagation_)
DECLARE_INSTRUCTION_SERIALIZABLE_FIELDS(RedefinitionInstr,
TemplateDefinition,
@ -4027,6 +4060,7 @@ class ConstantInstr : public TemplateDefinition<0, NoThrow, Pure> {
intptr_t pair_index = 0);
PRINT_OPERANDS_TO_SUPPORT
DECLARE_ATTRIBUTES_NAMED(("value"), (&value()))
#define FIELD_LIST(F) \
F(const Object&, value_) \

View file

@ -219,6 +219,26 @@ class IlTestPrinter : public AllStatic {
void WriteAttribute(const Slot* slot) { writer_->PrintValue(slot->Name()); }
void WriteAttribute(const Object* obj) {
if (obj->IsNull()) {
writer_->PrintValueNull();
} else if (obj->IsBool()) {
writer_->PrintValueBool(Bool::Cast(*obj).value());
} else if (obj->IsInteger()) {
auto value = Integer::Cast(*obj).AsInt64Value();
// PrintValue64 and PrintValue will check if integer is representable
// as a JS number, which is too strict. We don't actually need
// such checks because we only load resulting JSON in Dart.
writer_->buffer()->Printf("%" Pd64 "", value);
} else if (obj->IsString()) {
const auto& str = String::Cast(*obj);
writer_->PrintValueStr(str, 0, str.Length());
} else {
const auto& cls = Class::Handle(obj->clazz());
writer_->PrintfValue("Instance of %s", cls.UserVisibleNameCString());
}
}
template <typename... Ts>
void WriteTuple(const std::tuple<Ts...>& tuple) {
std::apply([&](Ts const&... elements) { WriteAttribute(elements...); },