Commit 8f09a751 authored by Jaroslav Sevcik's avatar Jaroslav Sevcik Committed by Commit Bot

Revert "[turbofan] Load elimination prunes control flow based on instance type."

This reverts commit 71bcc1d9.

Reason for revert: Regresses Octane/Box2D, among other things.

Original change's description:
> [turbofan] Load elimination prunes control flow based on instance type.
> 
> Changes:
> - introduce the notion of unreachable abstract states.
> 
> - reconnect unreachables states to runtime abort in effect phis (so that
>   the merged states are not polluted by unreachable branches while
>   preserving SSA).
> 
> - mark states with failed map checks, unreachable map guars as unreachable.
> 
> - add instance type to AbstractMaps, only invalidate instance type on
>   mismatched effect merges.
> 
> 
> This results in 2-3% improvement on ARES/ML steady state.
> 
> Bug: v8:6396
> Change-Id: I35b0d4482fa400ba7ee9a754f8ef1b2663ebc7dc
> Reviewed-on: https://chromium-review.googlesource.com/727761
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#48742}

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

Change-Id: I6302b37dbf5ea781c64815ef1900681531ad7d71
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:6396
Reviewed-on: https://chromium-review.googlesource.com/728440Reviewed-by: 's avatarJaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48763}
parent ff8b937f
......@@ -141,7 +141,6 @@ namespace internal {
V(kUnexpectedReturnFromFrameDropper, \
"Unexpectedly returned from dropping frames") \
V(kUnexpectedReturnFromThrow, "Unexpectedly returned from a throw") \
V(kUnreachableCodeReached, "Reached unreachable code") \
V(kVariableResolvedToWithContext, "Variable resolved to with context") \
V(kWithStatement, "WithStatement") \
V(kWrongFunctionContext, "Wrong context passed to function") \
......
......@@ -297,6 +297,11 @@ RegionObservability RegionObservabilityOf(Operator const* op) {
return OpParameter<RegionObservability>(op);
}
ZoneHandleSet<Map> MapGuardMapsOf(Operator const* op) {
DCHECK_EQ(IrOpcode::kMapGuard, op->opcode());
return OpParameter<ZoneHandleSet<Map>>(op);
}
Type* TypeGuardTypeOf(Operator const* op) {
DCHECK_EQ(IrOpcode::kTypeGuard, op->opcode());
return OpParameter<Type*>(op);
......@@ -1125,6 +1130,14 @@ const Operator* CommonOperatorBuilder::Phi(MachineRepresentation rep,
rep); // parameter
}
const Operator* CommonOperatorBuilder::MapGuard(ZoneHandleSet<Map> maps) {
return new (zone()) Operator1<ZoneHandleSet<Map>>( // --
IrOpcode::kMapGuard, Operator::kEliminatable, // opcode
"MapGuard", // name
1, 1, 1, 0, 1, 0, // counts
maps); // parameter
}
const Operator* CommonOperatorBuilder::TypeGuard(Type* type) {
return new (zone()) Operator1<Type*>( // --
IrOpcode::kTypeGuard, Operator::kPure, // opcode
......
......@@ -304,6 +304,8 @@ RegionObservability RegionObservabilityOf(Operator const*) WARN_UNUSED_RESULT;
std::ostream& operator<<(std::ostream& os,
const ZoneVector<MachineType>* types);
ZoneHandleSet<Map> MapGuardMapsOf(Operator const*) WARN_UNUSED_RESULT;
Type* TypeGuardTypeOf(Operator const*) WARN_UNUSED_RESULT;
int OsrValueIndexOf(Operator const*);
......@@ -417,6 +419,7 @@ class V8_EXPORT_PRIVATE CommonOperatorBuilder final
const Operator* TailCall(const CallDescriptor* descriptor);
const Operator* Projection(size_t index);
const Operator* Retain();
const Operator* MapGuard(ZoneHandleSet<Map> maps);
const Operator* TypeGuard(Type* type);
// Constructs a new merge or phi operator with the same opcode as {op}, but
......
......@@ -1330,7 +1330,7 @@ void EffectControlLinearizer::LowerCheckMaps(Node* node, Node* frame_state) {
}
Node* EffectControlLinearizer::LowerCompareMaps(Node* node) {
ZoneHandleSet<Map> const& maps = CompareMapsParametersOf(node->op()).maps();
ZoneHandleSet<Map> const& maps = CompareMapsParametersOf(node->op());
size_t const map_count = maps.size();
Node* value = node->InputAt(0);
......
......@@ -661,8 +661,8 @@ void ReduceNode(const Operator* op, EscapeAnalysisTracker::Scope* current,
if (Node* map = current->Get(map_field)) {
Type* const map_type = NodeProperties::GetType(map);
if (map_type->IsHeapConstant() &&
params.maps().contains(
bit_cast<Handle<Map>>(map_type->AsHeapConstant()->Value()))) {
params.maps().contains(ZoneHandleSet<Map>(bit_cast<Handle<Map>>(
map_type->AsHeapConstant()->Value())))) {
current->MarkForDeletion();
break;
}
......@@ -683,7 +683,7 @@ void ReduceNode(const Operator* op, EscapeAnalysisTracker::Scope* current,
vobject->FieldAt(HeapObject::kMapOffset).To(&map_field)) {
if (Node* object_map = current->Get(map_field)) {
current->SetReplacement(LowerCompareMapsWithoutLoad(
object_map, CompareMapsParametersOf(op).maps(), jsgraph));
object_map, CompareMapsParametersOf(op), jsgraph));
break;
} else {
// If the variable has no value, we have not reached the fixed-point
......
......@@ -840,7 +840,7 @@ Reduction JSNativeContextSpecialization::ReduceNamedAccess(
for (auto receiver_map : receiver_maps) {
maps.insert(receiver_map, graph()->zone());
}
this_effect = graph()->NewNode(simplified()->MapGuard(maps), receiver,
this_effect = graph()->NewNode(common()->MapGuard(maps), receiver,
this_effect, this_control);
}
}
......@@ -1196,7 +1196,7 @@ Reduction JSNativeContextSpecialization::ReduceElementAccess(
this_control = graph()->NewNode(common()->IfTrue(), branch);
// Introduce a MapGuard to learn from this on the effect chain.
this_effect = graph()->NewNode(simplified()->MapGuard(maps), receiver,
this_effect = graph()->NewNode(common()->MapGuard(maps), receiver,
this_effect, this_control);
}
......
This diff is collapsed.
......@@ -24,12 +24,12 @@ class CommonOperatorBuilder;
struct FieldAccess;
class Graph;
class JSGraph;
class MapsParameterInfo;
class V8_EXPORT_PRIVATE LoadElimination final
: public NON_EXPORTED_BASE(AdvancedReducer) {
public:
LoadElimination(Editor* editor, JSGraph* jsgraph, Zone* zone);
LoadElimination(Editor* editor, JSGraph* jsgraph, Zone* zone)
: AdvancedReducer(editor), node_states_(zone), jsgraph_(jsgraph) {}
~LoadElimination() final {}
const char* reducer_name() const override { return "LoadElimination"; }
......@@ -188,92 +188,49 @@ class V8_EXPORT_PRIVATE LoadElimination final
static size_t const kMaxTrackedFields = 32;
enum class MapSetComparisonResult {
kSubset,
kDisjoint,
kOther,
};
static MapSetComparisonResult CompareMapSets(ZoneHandleSet<Map> const& lhs,
ZoneHandleSet<Map> const& rhs);
class AbstractMapInfo {
public:
enum InfoValid { kMapsValid = 1 << 0, kInstanceTypeValid = 1 << 1 };
typedef base::Flags<InfoValid, uint8_t> Validity;
AbstractMapInfo();
explicit AbstractMapInfo(ZoneHandleSet<Map> const& maps);
explicit AbstractMapInfo(InstanceType instance_type);
explicit AbstractMapInfo(MapsParameterInfo const& info);
bool operator==(AbstractMapInfo const& other) const;
bool operator!=(AbstractMapInfo const& other) const;
ZoneHandleSet<Map> const& maps() const;
InstanceType instance_type() const;
bool maps_valid() const { return info_validity_ & kMapsValid; }
bool instance_type_valid() const {
return info_validity_ & kInstanceTypeValid;
}
bool empty() const { return info_validity_ == 0; }
AbstractMapInfo Merge(AbstractMapInfo const& other) const;
MapSetComparisonResult Compare(AbstractMapInfo const& other) const;
private:
AbstractMapInfo(ZoneHandleSet<Map> const& maps, InstanceType instance_type,
Validity validity);
ZoneHandleSet<Map> maps_;
InstanceType instance_type_;
Validity info_validity_;
};
// Abstract state to approximate the current map of an object along the
// effect paths through the graph.
class AbstractMaps final : public ZoneObject {
public:
explicit AbstractMaps(Zone* zone) : info_for_node_(zone) {}
AbstractMaps(Node* object, AbstractMapInfo const& map_info, Zone* zone)
AbstractMaps(Node* object, ZoneHandleSet<Map> maps, Zone* zone)
: info_for_node_(zone) {
info_for_node_.insert(std::make_pair(object, map_info));
info_for_node_.insert(std::make_pair(object, maps));
}
AbstractMaps const* Extend(Node* object, AbstractMapInfo const& maps,
AbstractMaps const* Extend(Node* object, ZoneHandleSet<Map> maps,
Zone* zone) const;
bool Lookup(Node* object, ZoneHandleSet<Map>* object_maps) const;
bool Lookup(Node* object, AbstractMapInfo* maps_info) const;
AbstractMaps const* Kill(const AliasStateInfo& alias_info,
Zone* zone) const;
bool Equals(AbstractMaps const* that) const {
return this == that || this->info_for_node_ == that->info_for_node_;
}
AbstractMaps const* Merge(AbstractMaps const* that, Zone* zone) const;
AbstractMaps const* KillMutableState(Zone* zone) const;
void Print() const;
private:
ZoneMap<Node*, AbstractMapInfo> info_for_node_;
ZoneMap<Node*, ZoneHandleSet<Map>> info_for_node_;
};
class AbstractState final : public ZoneObject {
public:
AbstractState() {
for (size_t i = 0; i < arraysize(fields_); ++i) {
fields_[i] = nullptr;
}
}
bool Equals(AbstractState const* that) const;
void Merge(AbstractState const* that, Zone* zone);
AbstractState const* AddMaps(Node* object, ZoneHandleSet<Map> maps,
Zone* zone) const;
AbstractState const* AddMaps(Node* object, AbstractMapInfo const& map_info,
Zone* zone) const;
AbstractState const* KillMaps(Node* object, Zone* zone) const;
AbstractState const* KillMaps(const AliasStateInfo& alias_info,
Zone* zone) const;
bool LookupMaps(Node* object, ZoneHandleSet<Map>* object_maps) const;
bool LookupMaps(Node* object, AbstractMapInfo* object_maps) const;
AbstractState const* AddField(Node* object, size_t index, Node* value,
MaybeHandle<Name> name, Zone* zone) const;
......@@ -297,28 +254,13 @@ class V8_EXPORT_PRIVATE LoadElimination final
AbstractState const* AddCheck(Node* node, Zone* zone) const;
Node* LookupCheck(Node* node) const;
AbstractState const* KillMutableState(Zone* zone) const;
bool is_unreachable() const { return is_unreachable_; }
void Print() const;
static AbstractState Unreachable();
static AbstractState Empty();
private:
AbstractState() : is_unreachable_(false) {
for (size_t i = 0; i < arraysize(fields_); ++i) {
fields_[i] = nullptr;
}
}
AbstractChecks const* checks_ = nullptr;
AbstractElements const* elements_ = nullptr;
AbstractField const* fields_[kMaxTrackedFields];
AbstractMaps const* maps_ = nullptr;
bool is_unreachable_;
};
class AbstractStateForEffectNodes final : public ZoneObject {
......@@ -362,14 +304,12 @@ class V8_EXPORT_PRIVATE LoadElimination final
CommonOperatorBuilder* common() const;
AbstractState const* empty_state() const { return &empty_state_; }
AbstractState const* unreachable_state() const { return &unreachable_state_; }
Factory* factory() const;
Graph* graph() const;
JSGraph* jsgraph() const { return jsgraph_; }
Zone* zone() const { return node_states_.zone(); }
AbstractState const empty_state_;
AbstractState const unreachable_state_;
AbstractStateForEffectNodes node_states_;
JSGraph* const jsgraph_;
......
......@@ -362,7 +362,7 @@ NodeProperties::InferReceiverMapsResult NodeProperties::InferReceiverMaps(
case IrOpcode::kMapGuard: {
Node* const object = GetValueInput(effect, 0);
if (IsSame(receiver, object)) {
*maps_return = MapGuardMapsOf(effect->op()).maps();
*maps_return = MapGuardMapsOf(effect->op());
return result;
}
break;
......
......@@ -179,40 +179,6 @@ std::ostream& operator<<(std::ostream& os, CheckMapsFlags flags) {
return os;
}
MapsParameterInfo::MapsParameterInfo(ZoneHandleSet<Map> const& maps)
: maps_(maps), instance_type_(Nothing<InstanceType>()) {
DCHECK_LT(0, maps.size());
instance_type_ = Just(maps.at(0)->instance_type());
for (size_t i = 1; i < maps.size(); ++i) {
if (instance_type_.FromJust() != maps.at(i)->instance_type()) {
instance_type_ = Nothing<InstanceType>();
break;
}
}
}
std::ostream& operator<<(std::ostream& os, MapsParameterInfo const& p) {
ZoneHandleSet<Map> const& maps = p.maps();
InstanceType instance_type;
if (p.instance_type().To(&instance_type)) {
os << ", " << instance_type;
}
for (size_t i = 0; i < maps.size(); ++i) {
os << ", " << Brief(*maps[i]);
}
return os;
}
bool operator==(MapsParameterInfo const& lhs, MapsParameterInfo const& rhs) {
return lhs.maps() == rhs.maps();
}
bool operator!=(MapsParameterInfo const& lhs, MapsParameterInfo const& rhs) {
return !(lhs == rhs);
}
size_t hash_value(MapsParameterInfo const& p) { return hash_value(p.maps()); }
bool operator==(CheckMapsParameters const& lhs,
CheckMapsParameters const& rhs) {
return lhs.flags() == rhs.flags() && lhs.maps() == rhs.maps();
......@@ -223,7 +189,12 @@ size_t hash_value(CheckMapsParameters const& p) {
}
std::ostream& operator<<(std::ostream& os, CheckMapsParameters const& p) {
return os << p.flags() << p.maps_info();
ZoneHandleSet<Map> const& maps = p.maps();
os << p.flags();
for (size_t i = 0; i < maps.size(); ++i) {
os << ", " << Brief(*maps[i]);
}
return os;
}
CheckMapsParameters const& CheckMapsParametersOf(Operator const* op) {
......@@ -231,14 +202,9 @@ CheckMapsParameters const& CheckMapsParametersOf(Operator const* op) {
return OpParameter<CheckMapsParameters>(op);
}
MapsParameterInfo const& CompareMapsParametersOf(Operator const* op) {
ZoneHandleSet<Map> const& CompareMapsParametersOf(Operator const* op) {
DCHECK_EQ(IrOpcode::kCompareMaps, op->opcode());
return OpParameter<MapsParameterInfo>(op);
}
MapsParameterInfo const& MapGuardMapsOf(Operator const* op) {
DCHECK_EQ(IrOpcode::kMapGuard, op->opcode());
return OpParameter<MapsParameterInfo>(op);
return OpParameter<ZoneHandleSet<Map>>(op);
}
size_t hash_value(CheckTaggedInputMode mode) {
......@@ -1006,22 +972,14 @@ const Operator* SimplifiedOperatorBuilder::CheckMaps(CheckMapsFlags flags,
parameters); // parameter
}
const Operator* SimplifiedOperatorBuilder::MapGuard(ZoneHandleSet<Map> maps) {
return new (zone()) Operator1<MapsParameterInfo>( // --
IrOpcode::kMapGuard, Operator::kEliminatable, // opcode
"MapGuard", // name
1, 1, 1, 0, 1, 0, // counts
MapsParameterInfo(maps)); // parameter
}
const Operator* SimplifiedOperatorBuilder::CompareMaps(
ZoneHandleSet<Map> maps) {
return new (zone()) Operator1<MapsParameterInfo>( // --
IrOpcode::kCompareMaps, // opcode
Operator::kEliminatable, // flags
"CompareMaps", // name
1, 1, 1, 1, 1, 0, // counts
MapsParameterInfo(maps)); // parameter
return new (zone()) Operator1<ZoneHandleSet<Map>>( // --
IrOpcode::kCompareMaps, // opcode
Operator::kEliminatable, // flags
"CompareMaps", // name
1, 1, 1, 1, 1, 0, // counts
maps); // parameter
}
const Operator* SimplifiedOperatorBuilder::CheckFloat64Hole(
......
......@@ -133,38 +133,18 @@ DEFINE_OPERATORS_FOR_FLAGS(CheckMapsFlags)
std::ostream& operator<<(std::ostream&, CheckMapsFlags);
class MapsParameterInfo {
public:
explicit MapsParameterInfo(ZoneHandleSet<Map> const& maps);
Maybe<InstanceType> instance_type() const { return instance_type_; }
ZoneHandleSet<Map> const& maps() const { return maps_; }
private:
ZoneHandleSet<Map> const maps_;
Maybe<InstanceType> instance_type_;
};
std::ostream& operator<<(std::ostream&, MapsParameterInfo const&);
bool operator==(MapsParameterInfo const&, MapsParameterInfo const&);
bool operator!=(MapsParameterInfo const&, MapsParameterInfo const&);
size_t hash_value(MapsParameterInfo const&);
// A descriptor for map checks.
class CheckMapsParameters final {
public:
CheckMapsParameters(CheckMapsFlags flags, ZoneHandleSet<Map> const& maps)
: flags_(flags), maps_info_(maps) {}
: flags_(flags), maps_(maps) {}
CheckMapsFlags flags() const { return flags_; }
ZoneHandleSet<Map> const& maps() const { return maps_info_.maps(); }
MapsParameterInfo const& maps_info() const { return maps_info_; }
ZoneHandleSet<Map> const& maps() const { return maps_; }
private:
CheckMapsFlags const flags_;
MapsParameterInfo const maps_info_;
ZoneHandleSet<Map> const maps_;
};
bool operator==(CheckMapsParameters const&, CheckMapsParameters const&);
......@@ -176,10 +156,8 @@ std::ostream& operator<<(std::ostream&, CheckMapsParameters const&);
CheckMapsParameters const& CheckMapsParametersOf(Operator const*)
WARN_UNUSED_RESULT;
MapsParameterInfo const& MapGuardMapsOf(Operator const*) WARN_UNUSED_RESULT;
// Parameters for CompareMaps operator.
MapsParameterInfo const& CompareMapsParametersOf(Operator const*)
ZoneHandleSet<Map> const& CompareMapsParametersOf(Operator const*)
WARN_UNUSED_RESULT;
// A descriptor for growing elements backing stores.
......@@ -437,7 +415,6 @@ class V8_EXPORT_PRIVATE SimplifiedOperatorBuilder final
const Operator* CheckBounds();
const Operator* CheckMaps(CheckMapsFlags, ZoneHandleSet<Map>);
const Operator* CompareMaps(ZoneHandleSet<Map>);
const Operator* MapGuard(ZoneHandleSet<Map> maps);
const Operator* CheckHeapObject();
const Operator* CheckInternalizedString();
......
......@@ -96,15 +96,6 @@ class ZoneHandleSet final {
return true;
}
bool contains(Handle<T> other) const {
if (data_ == kEmptyTag) return false;
if ((data_ & kTagMask) == kSingletonTag) {
return singleton() == bit_cast<T**>(other.address());
}
DCHECK_EQ(kListTag, data_ & kTagMask);
return list()->Contains(bit_cast<T**>(other.address()));
}
void remove(Handle<T> handle, Zone* zone) {
// TODO(bmeurer): Optimize this case.
ZoneHandleSet<T> that;
......
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