Commit 42ed37d0 authored by Santiago Aboy Solanes's avatar Santiago Aboy Solanes Committed by Commit Bot

[ptr-compr][cleanup] Removing the optimized map checks

The DecompressionElimination reducer can handle that case with the
comparison of Decompress vs HeapConstant. There is no need to do extra
work.

Reverts parts of https://chromium-review.googlesource.com/c/v8/v8/+/1518182.
The rest of that CL was reverted in a previous CL where the AccessBuilders
were updated.

Cq-Include-Trybots: luci.v8.try:v8_linux64_pointer_compression_rel_ng
Cq-Include-Trybots: luci.v8.try:v8_linux64_arm64_pointer_compression_rel_ng
Bug: v8:8977, v8:7703
Change-Id: I871577e49f9ccd95864af54bdd61884d34b7f223
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1628792Reviewed-by: 's avatarMichael Stanton <mvstanton@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61883}
parent 9ac15080
...@@ -26,18 +26,13 @@ namespace v8 { ...@@ -26,18 +26,13 @@ namespace v8 {
namespace internal { namespace internal {
namespace compiler { namespace compiler {
namespace {
bool UsingCompressedPointers() { return false; }
} // namespace
class EffectControlLinearizer { class EffectControlLinearizer {
public: public:
EffectControlLinearizer(JSGraph* js_graph, Schedule* schedule, EffectControlLinearizer(JSGraph* js_graph, Schedule* schedule,
Zone* temp_zone, Zone* temp_zone,
SourcePositionTable* source_positions, SourcePositionTable* source_positions,
NodeOriginTable* node_origins, NodeOriginTable* node_origins,
MaskArrayIndexEnable mask_array_index, MaskArrayIndexEnable mask_array_index)
std::vector<Handle<Map>>* embedded_maps)
: js_graph_(js_graph), : js_graph_(js_graph),
schedule_(schedule), schedule_(schedule),
temp_zone_(temp_zone), temp_zone_(temp_zone),
...@@ -45,8 +40,7 @@ class EffectControlLinearizer { ...@@ -45,8 +40,7 @@ class EffectControlLinearizer {
source_positions_(source_positions), source_positions_(source_positions),
node_origins_(node_origins), node_origins_(node_origins),
graph_assembler_(js_graph, nullptr, nullptr, temp_zone), graph_assembler_(js_graph, nullptr, nullptr, temp_zone),
frame_state_zapper_(nullptr), frame_state_zapper_(nullptr) {}
embedded_maps_(embedded_maps) {}
void Run(); void Run();
...@@ -249,7 +243,6 @@ class EffectControlLinearizer { ...@@ -249,7 +243,6 @@ class EffectControlLinearizer {
return js_graph_->simplified(); return js_graph_->simplified();
} }
MachineOperatorBuilder* machine() const { return js_graph_->machine(); } MachineOperatorBuilder* machine() const { return js_graph_->machine(); }
std::vector<Handle<Map>>* embedded_maps() { return embedded_maps_; }
GraphAssembler* gasm() { return &graph_assembler_; } GraphAssembler* gasm() { return &graph_assembler_; }
JSGraph* js_graph_; JSGraph* js_graph_;
...@@ -261,9 +254,6 @@ class EffectControlLinearizer { ...@@ -261,9 +254,6 @@ class EffectControlLinearizer {
NodeOriginTable* node_origins_; NodeOriginTable* node_origins_;
GraphAssembler graph_assembler_; GraphAssembler graph_assembler_;
Node* frame_state_zapper_; // For tracking down compiler::Node::New crashes. Node* frame_state_zapper_; // For tracking down compiler::Node::New crashes.
// {embedded_maps_} keeps track of maps we've embedded as Uint32 constants.
// We do this in order to notify the garbage collector at code-gen time.
std::vector<Handle<Map>>* embedded_maps_;
}; };
namespace { namespace {
...@@ -1811,20 +1801,8 @@ void EffectControlLinearizer::LowerCheckMaps(Node* node, Node* frame_state) { ...@@ -1811,20 +1801,8 @@ void EffectControlLinearizer::LowerCheckMaps(Node* node, Node* frame_state) {
Node* value_map = __ LoadField(AccessBuilder::ForMap(), value); Node* value_map = __ LoadField(AccessBuilder::ForMap(), value);
for (size_t i = 0; i < map_count; ++i) { for (size_t i = 0; i < map_count; ++i) {
Node* check; Node* map = __ HeapConstant(maps[i]);
Node* check = __ WordEqual(value_map, map);
if (UsingCompressedPointers()) {
// We need the dereference scope to embed the map pointer value as an
// int32. We don't visit the pointer.
AllowHandleDereference allow_map_dereference;
int32_t int32Map = static_cast<int32_t>(CompressTagged(maps[i]->ptr()));
Node* map = __ Int32Constant(int32Map);
check = __ Word32Equal(value_map, map);
this->embedded_maps()->push_back(maps[i]);
} else {
Node* map = __ HeapConstant(maps[i]);
check = __ WordEqual(value_map, map);
}
if (i == map_count - 1) { if (i == map_count - 1) {
__ DeoptimizeIfNot(DeoptimizeReason::kWrongMap, p.feedback(), check, __ DeoptimizeIfNot(DeoptimizeReason::kWrongMap, p.feedback(), check,
...@@ -1851,20 +1829,8 @@ Node* EffectControlLinearizer::LowerCompareMaps(Node* node) { ...@@ -1851,20 +1829,8 @@ Node* EffectControlLinearizer::LowerCompareMaps(Node* node) {
Node* value_map = __ LoadField(AccessBuilder::ForMap(), value); Node* value_map = __ LoadField(AccessBuilder::ForMap(), value);
for (size_t i = 0; i < map_count; ++i) { for (size_t i = 0; i < map_count; ++i) {
Node* check; Node* map = __ HeapConstant(maps[i]);
Node* check = __ WordEqual(value_map, map);
if (UsingCompressedPointers()) {
// We need the dereference scope to embed the map pointer value as an
// int32. We don't visit the pointer.
AllowHandleDereference allow_map_dereference;
int32_t int32Map = static_cast<int32_t>(CompressTagged(maps[i]->ptr()));
Node* map = __ Int32Constant(int32Map);
check = __ Word32Equal(value_map, map);
this->embedded_maps()->push_back(maps[i]);
} else {
Node* map = __ HeapConstant(maps[i]);
check = __ WordEqual(value_map, map);
}
auto next_map = __ MakeLabel(); auto next_map = __ MakeLabel();
auto passed = __ MakeLabel(); auto passed = __ MakeLabel();
...@@ -5733,11 +5699,10 @@ Node* EffectControlLinearizer::LowerDateNow(Node* node) { ...@@ -5733,11 +5699,10 @@ Node* EffectControlLinearizer::LowerDateNow(Node* node) {
void LinearizeEffectControl(JSGraph* graph, Schedule* schedule, Zone* temp_zone, void LinearizeEffectControl(JSGraph* graph, Schedule* schedule, Zone* temp_zone,
SourcePositionTable* source_positions, SourcePositionTable* source_positions,
NodeOriginTable* node_origins, NodeOriginTable* node_origins,
MaskArrayIndexEnable mask_array_index, MaskArrayIndexEnable mask_array_index) {
std::vector<Handle<Map>>* embedded_maps) {
EffectControlLinearizer linearizer(graph, schedule, temp_zone, EffectControlLinearizer linearizer(graph, schedule, temp_zone,
source_positions, node_origins, source_positions, node_origins,
mask_array_index, embedded_maps); mask_array_index);
linearizer.Run(); linearizer.Run();
} }
......
...@@ -27,8 +27,7 @@ enum class MaskArrayIndexEnable { kDoNotMaskArrayIndex, kMaskArrayIndex }; ...@@ -27,8 +27,7 @@ enum class MaskArrayIndexEnable { kDoNotMaskArrayIndex, kMaskArrayIndex };
V8_EXPORT_PRIVATE void LinearizeEffectControl( V8_EXPORT_PRIVATE void LinearizeEffectControl(
JSGraph* graph, Schedule* schedule, Zone* temp_zone, JSGraph* graph, Schedule* schedule, Zone* temp_zone,
SourcePositionTable* source_positions, NodeOriginTable* node_origins, SourcePositionTable* source_positions, NodeOriginTable* node_origins,
MaskArrayIndexEnable mask_array_index, MaskArrayIndexEnable mask_array_index);
std::vector<Handle<Map>>* embedded_maps);
} // namespace compiler } // namespace compiler
} // namespace internal } // namespace internal
......
...@@ -291,7 +291,6 @@ class PipelineData { ...@@ -291,7 +291,6 @@ class PipelineData {
Zone* codegen_zone() const { return codegen_zone_; } Zone* codegen_zone() const { return codegen_zone_; }
InstructionSequence* sequence() const { return sequence_; } InstructionSequence* sequence() const { return sequence_; }
Frame* frame() const { return frame_; } Frame* frame() const { return frame_; }
std::vector<Handle<Map>>* embedded_maps() { return &embedded_maps_; }
Zone* register_allocation_zone() const { return register_allocation_zone_; } Zone* register_allocation_zone() const { return register_allocation_zone_; }
RegisterAllocationData* register_allocation_data() const { RegisterAllocationData* register_allocation_data() const {
...@@ -495,10 +494,6 @@ class PipelineData { ...@@ -495,10 +494,6 @@ class PipelineData {
JSHeapBroker* broker_ = nullptr; JSHeapBroker* broker_ = nullptr;
Frame* frame_ = nullptr; Frame* frame_ = nullptr;
// embedded_maps_ keeps track of maps we've embedded as Uint32 constants.
// We do this in order to notify the garbage collector at code-gen time.
std::vector<Handle<Map>> embedded_maps_;
// All objects in the following group of fields are allocated in // All objects in the following group of fields are allocated in
// register_allocation_zone_. They are all set to nullptr when the zone is // register_allocation_zone_. They are all set to nullptr when the zone is
// destroyed. // destroyed.
...@@ -1023,8 +1018,8 @@ PipelineCompilationJob::Status PipelineCompilationJob::FinalizeJobImpl( ...@@ -1023,8 +1018,8 @@ PipelineCompilationJob::Status PipelineCompilationJob::FinalizeJobImpl(
void PipelineCompilationJob::RegisterWeakObjectsInOptimizedCode( void PipelineCompilationJob::RegisterWeakObjectsInOptimizedCode(
Handle<Code> code, Isolate* isolate) { Handle<Code> code, Isolate* isolate) {
std::vector<Handle<Map>> maps;
DCHECK(code->is_optimized_code()); DCHECK(code->is_optimized_code());
std::vector<Handle<Map>> retained_maps;
{ {
DisallowHeapAllocation no_gc; DisallowHeapAllocation no_gc;
int const mode_mask = RelocInfo::EmbeddedObjectModeMask(); int const mode_mask = RelocInfo::EmbeddedObjectModeMask();
...@@ -1034,23 +1029,14 @@ void PipelineCompilationJob::RegisterWeakObjectsInOptimizedCode( ...@@ -1034,23 +1029,14 @@ void PipelineCompilationJob::RegisterWeakObjectsInOptimizedCode(
Handle<HeapObject> object(HeapObject::cast(it.rinfo()->target_object()), Handle<HeapObject> object(HeapObject::cast(it.rinfo()->target_object()),
isolate); isolate);
if (object->IsMap()) { if (object->IsMap()) {
retained_maps.push_back(Handle<Map>::cast(object)); maps.push_back(Handle<Map>::cast(object));
} }
} }
} }
} }
for (Handle<Map> map : maps) {
for (Handle<Map> map : retained_maps) {
isolate->heap()->AddRetainedMap(map); isolate->heap()->AddRetainedMap(map);
} }
// Additionally, gather embedded maps if we have any.
for (Handle<Map> map : *data_.embedded_maps()) {
if (code->IsWeakObjectInOptimizedCode(*map)) {
isolate->heap()->AddRetainedMap(map);
}
}
code->set_can_have_weak_objects(true); code->set_can_have_weak_objects(true);
} }
...@@ -1438,7 +1424,7 @@ struct EffectControlLinearizationPhase { ...@@ -1438,7 +1424,7 @@ struct EffectControlLinearizationPhase {
// - introduce effect phis and rewire effects to get SSA again. // - introduce effect phis and rewire effects to get SSA again.
LinearizeEffectControl(data->jsgraph(), schedule, temp_zone, LinearizeEffectControl(data->jsgraph(), schedule, temp_zone,
data->source_positions(), data->node_origins(), data->source_positions(), data->node_origins(),
mask_array_index, data->embedded_maps()); mask_array_index);
} }
{ {
// The {EffectControlLinearizer} might leave {Dead} nodes behind, so we // The {EffectControlLinearizer} might leave {Dead} nodes behind, so we
......
...@@ -40,7 +40,6 @@ class EffectControlLinearizerTest : public GraphTest { ...@@ -40,7 +40,6 @@ class EffectControlLinearizerTest : public GraphTest {
SimplifiedOperatorBuilder* simplified() { return &simplified_; } SimplifiedOperatorBuilder* simplified() { return &simplified_; }
SourcePositionTable* source_positions() { return source_positions_; } SourcePositionTable* source_positions() { return source_positions_; }
NodeOriginTable* node_origins() { return node_origins_; } NodeOriginTable* node_origins() { return node_origins_; }
std::vector<Handle<Map>>* maps() { return &maps_; }
private: private:
MachineOperatorBuilder machine_; MachineOperatorBuilder machine_;
...@@ -49,7 +48,6 @@ class EffectControlLinearizerTest : public GraphTest { ...@@ -49,7 +48,6 @@ class EffectControlLinearizerTest : public GraphTest {
JSGraph jsgraph_; JSGraph jsgraph_;
SourcePositionTable* source_positions_; SourcePositionTable* source_positions_;
NodeOriginTable* node_origins_; NodeOriginTable* node_origins_;
std::vector<Handle<Map>> maps_;
}; };
namespace { namespace {
...@@ -89,7 +87,7 @@ TEST_F(EffectControlLinearizerTest, SimpleLoad) { ...@@ -89,7 +87,7 @@ TEST_F(EffectControlLinearizerTest, SimpleLoad) {
// Run the state effect introducer. // Run the state effect introducer.
LinearizeEffectControl(jsgraph(), &schedule, zone(), source_positions(), LinearizeEffectControl(jsgraph(), &schedule, zone(), source_positions(),
node_origins(), node_origins(),
MaskArrayIndexEnable::kDoNotMaskArrayIndex, maps()); MaskArrayIndexEnable::kDoNotMaskArrayIndex);
EXPECT_THAT(load, EXPECT_THAT(load,
IsLoadField(AccessBuilder::ForHeapNumberValue(), heap_number, IsLoadField(AccessBuilder::ForHeapNumberValue(), heap_number,
...@@ -151,7 +149,7 @@ TEST_F(EffectControlLinearizerTest, DiamondLoad) { ...@@ -151,7 +149,7 @@ TEST_F(EffectControlLinearizerTest, DiamondLoad) {
// Run the state effect introducer. // Run the state effect introducer.
LinearizeEffectControl(jsgraph(), &schedule, zone(), source_positions(), LinearizeEffectControl(jsgraph(), &schedule, zone(), source_positions(),
node_origins(), node_origins(),
MaskArrayIndexEnable::kDoNotMaskArrayIndex, maps()); MaskArrayIndexEnable::kDoNotMaskArrayIndex);
// The effect input to the return should be an effect phi with the // The effect input to the return should be an effect phi with the
// newly introduced effectful change operators. // newly introduced effectful change operators.
...@@ -218,7 +216,7 @@ TEST_F(EffectControlLinearizerTest, LoopLoad) { ...@@ -218,7 +216,7 @@ TEST_F(EffectControlLinearizerTest, LoopLoad) {
// Run the state effect introducer. // Run the state effect introducer.
LinearizeEffectControl(jsgraph(), &schedule, zone(), source_positions(), LinearizeEffectControl(jsgraph(), &schedule, zone(), source_positions(),
node_origins(), node_origins(),
MaskArrayIndexEnable::kDoNotMaskArrayIndex, maps()); MaskArrayIndexEnable::kDoNotMaskArrayIndex);
ASSERT_THAT(ret, IsReturn(load, load, if_true)); ASSERT_THAT(ret, IsReturn(load, load, if_true));
EXPECT_THAT(load, IsLoadField(AccessBuilder::ForHeapNumberValue(), EXPECT_THAT(load, IsLoadField(AccessBuilder::ForHeapNumberValue(),
...@@ -281,7 +279,7 @@ TEST_F(EffectControlLinearizerTest, CloneBranch) { ...@@ -281,7 +279,7 @@ TEST_F(EffectControlLinearizerTest, CloneBranch) {
LinearizeEffectControl(jsgraph(), &schedule, zone(), source_positions(), LinearizeEffectControl(jsgraph(), &schedule, zone(), source_positions(),
node_origins(), node_origins(),
MaskArrayIndexEnable::kDoNotMaskArrayIndex, maps()); MaskArrayIndexEnable::kDoNotMaskArrayIndex);
Capture<Node *> branch1_capture, branch2_capture; Capture<Node *> branch1_capture, branch2_capture;
EXPECT_THAT( EXPECT_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