From d2528de4785c39bb56b54f3da9ee3e58cd53d98d Mon Sep 17 00:00:00 2001 From: Tobias Tebbi Date: Sat, 14 Dec 2019 20:04:20 +0100 Subject: [PATCH] Reland "[csa] use JSGraph to create constants in CodeAssembler" This is a reland of 53308bf7c0d0a7ff0f91364533c5edf992f51101 Original change's description: > [csa] use JSGraph to create constants in CodeAssembler > > Now that CodeAssembler uses optimizing TurboFan passes, creating > constants without using the caching implemented in JSGraph leads to > problems, since value numbering only works properly if all constants > in the graph were introduced through the cache. > To mitigate this, this CL creates the JSGraph earlier so that > CodeAssembler can already use the same JSGraph used by later TurboFan > optimizations. > For other uses of RawMachineAssembler, everything stays as before. > > This issue is creating bot failures in > https://chromium-review.googlesource.com/c/v8/v8/+/1958011 > > Change-Id: Ife017876b19cb2602694279ef1da75f23e18a031 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1967329 > Reviewed-by: Maya Lekova > Commit-Queue: Tobias Tebbi > Cr-Commit-Position: refs/heads/master@{#65477} TBR=mslekova@chromium.org Change-Id: I5c8218ce22470b3efa06d872176c910a4c5325a4 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1977858 Reviewed-by: Tobias Tebbi Commit-Queue: Tobias Tebbi Cr-Commit-Position: refs/heads/master@{#65537} --- src/compiler/code-assembler.cc | 23 ++++++++++++------- src/compiler/code-assembler.h | 3 +++ src/compiler/pipeline.cc | 40 +++++++++++++++++++++------------- src/compiler/pipeline.h | 3 ++- 4 files changed, 45 insertions(+), 24 deletions(-) diff --git a/src/compiler/code-assembler.cc b/src/compiler/code-assembler.cc index 44a704aab7..9d3ba4bada 100644 --- a/src/compiler/code-assembler.cc +++ b/src/compiler/code-assembler.cc @@ -13,6 +13,7 @@ #include "src/codegen/macro-assembler.h" #include "src/compiler/backend/instruction-selector.h" #include "src/compiler/graph.h" +#include "src/compiler/js-graph.h" #include "src/compiler/linkage.h" #include "src/compiler/node-matchers.h" #include "src/compiler/pipeline.h" @@ -84,7 +85,11 @@ CodeAssemblerState::CodeAssemblerState(Isolate* isolate, Zone* zone, name_(name), builtin_index_(builtin_index), code_generated_(false), - variables_(zone) {} + variables_(zone), + jsgraph_(new (zone) JSGraph( + isolate, raw_assembler_->graph(), raw_assembler_->common(), + new (zone) JSOperatorBuilder(zone), raw_assembler_->simplified(), + raw_assembler_->machine())) {} CodeAssemblerState::~CodeAssemblerState() = default; @@ -180,7 +185,7 @@ Handle CodeAssembler::GenerateCode(CodeAssemblerState* state, Graph* graph = rasm->ExportForOptimization(); code = Pipeline::GenerateCodeForCodeStub( - rasm->isolate(), rasm->call_descriptor(), graph, + rasm->isolate(), rasm->call_descriptor(), graph, state->jsgraph_, rasm->source_positions(), state->kind_, state->name_, state->builtin_index_, rasm->poisoning_level(), options) .ToHandleChecked(); @@ -241,15 +246,15 @@ void CodeAssembler::GenerateCheckMaybeObjectIsObject(Node* node, #endif TNode CodeAssembler::Int32Constant(int32_t value) { - return UncheckedCast(raw_assembler()->Int32Constant(value)); + return UncheckedCast(jsgraph()->Int32Constant(value)); } TNode CodeAssembler::Int64Constant(int64_t value) { - return UncheckedCast(raw_assembler()->Int64Constant(value)); + return UncheckedCast(jsgraph()->Int64Constant(value)); } TNode CodeAssembler::IntPtrConstant(intptr_t value) { - return UncheckedCast(raw_assembler()->IntPtrConstant(value)); + return UncheckedCast(jsgraph()->IntPtrConstant(value)); } TNode CodeAssembler::NumberConstant(double value) { @@ -277,7 +282,7 @@ TNode CodeAssembler::SmiConstant(int value) { TNode CodeAssembler::UntypedHeapConstant( Handle object) { - return UncheckedCast(raw_assembler()->HeapConstant(object)); + return UncheckedCast(jsgraph()->HeapConstant(object)); } TNode CodeAssembler::StringConstant(const char* str) { @@ -289,7 +294,7 @@ TNode CodeAssembler::StringConstant(const char* str) { TNode CodeAssembler::BooleanConstant(bool value) { Handle object = isolate()->factory()->ToBoolean(value); return UncheckedCast( - raw_assembler()->HeapConstant(Handle::cast(object))); + jsgraph()->HeapConstant(Handle::cast(object))); } TNode CodeAssembler::ExternalConstant( @@ -299,7 +304,7 @@ TNode CodeAssembler::ExternalConstant( } TNode CodeAssembler::Float64Constant(double value) { - return UncheckedCast(raw_assembler()->Float64Constant(value)); + return UncheckedCast(jsgraph()->Float64Constant(value)); } bool CodeAssembler::ToInt32Constant(Node* node, int32_t* out_value) { @@ -1608,6 +1613,8 @@ RawMachineAssembler* CodeAssembler::raw_assembler() const { return state_->raw_assembler_.get(); } +JSGraph* CodeAssembler::jsgraph() const { return state_->jsgraph_; } + // The core implementation of Variable is stored through an indirection so // that it can outlive the often block-scoped Variable declarations. This is // needed to ensure that variable binding and merging through phis can diff --git a/src/compiler/code-assembler.h b/src/compiler/code-assembler.h index e7aa24c540..18b2cfd93d 100644 --- a/src/compiler/code-assembler.h +++ b/src/compiler/code-assembler.h @@ -187,6 +187,7 @@ class CodeAssemblerVariable; template class TypedCodeAssemblerVariable; class CodeAssemblerState; +class JSGraph; class Node; class RawMachineAssembler; class RawMachineLabel; @@ -1183,6 +1184,7 @@ class V8_EXPORT_PRIVATE CodeAssembler { TNode Unsigned(TNode x); RawMachineAssembler* raw_assembler() const; + JSGraph* jsgraph() const; // Calls respective callback registered in the state. void CallPrologue(); @@ -1437,6 +1439,7 @@ class V8_EXPORT_PRIVATE CodeAssemblerState { std::vector exception_handler_labels_; using VariableId = uint32_t; VariableId next_variable_id_ = 0; + JSGraph* jsgraph_; VariableId NextVariableId() { return next_variable_id_++; } diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc index bb4eaa8e7e..643c0d8dc6 100644 --- a/src/compiler/pipeline.cc +++ b/src/compiler/pipeline.cc @@ -209,7 +209,8 @@ class PipelineData { // For CodeStubAssembler and machine graph testing entry point. PipelineData(ZoneStats* zone_stats, OptimizedCompilationInfo* info, Isolate* isolate, AccountingAllocator* allocator, Graph* graph, - Schedule* schedule, SourcePositionTable* source_positions, + JSGraph* jsgraph, Schedule* schedule, + SourcePositionTable* source_positions, NodeOriginTable* node_origins, JumpOptimizationInfo* jump_opt, const AssemblerOptions& assembler_options) : isolate_(isolate), @@ -232,15 +233,23 @@ class PipelineData { register_allocation_zone_(register_allocation_zone_scope_.zone()), jump_optimization_info_(jump_opt), assembler_options_(assembler_options) { - simplified_ = new (graph_zone_) SimplifiedOperatorBuilder(graph_zone_); - machine_ = new (graph_zone_) MachineOperatorBuilder( - graph_zone_, MachineType::PointerRepresentation(), - InstructionSelector::SupportedMachineOperatorFlags(), - InstructionSelector::AlignmentRequirements()); - common_ = new (graph_zone_) CommonOperatorBuilder(graph_zone_); - javascript_ = new (graph_zone_) JSOperatorBuilder(graph_zone_); - jsgraph_ = new (graph_zone_) - JSGraph(isolate_, graph_, common_, javascript_, simplified_, machine_); + if (jsgraph) { + jsgraph_ = jsgraph; + simplified_ = jsgraph->simplified(); + machine_ = jsgraph->machine(); + common_ = jsgraph->common(); + javascript_ = jsgraph->javascript(); + } else { + simplified_ = new (graph_zone_) SimplifiedOperatorBuilder(graph_zone_); + machine_ = new (graph_zone_) MachineOperatorBuilder( + graph_zone_, MachineType::PointerRepresentation(), + InstructionSelector::SupportedMachineOperatorFlags(), + InstructionSelector::AlignmentRequirements()); + common_ = new (graph_zone_) CommonOperatorBuilder(graph_zone_); + javascript_ = new (graph_zone_) JSOperatorBuilder(graph_zone_); + jsgraph_ = new (graph_zone_) JSGraph(isolate_, graph_, common_, + javascript_, simplified_, machine_); + } } // For register allocation testing entry point. @@ -1186,7 +1195,7 @@ class WasmHeapStubCompilationJob final : public OptimizedCompilationJob { zone_(std::move(zone)), graph_(graph), data_(&zone_stats_, &info_, isolate, wasm_engine->allocator(), graph_, - nullptr, source_positions, + nullptr, nullptr, source_positions, new (zone_.get()) NodeOriginTable(graph_), nullptr, options), pipeline_(&data_), wasm_engine_(wasm_engine) {} @@ -2566,7 +2575,7 @@ bool PipelineImpl::OptimizeGraphForMidTier(Linkage* linkage) { MaybeHandle Pipeline::GenerateCodeForCodeStub( Isolate* isolate, CallDescriptor* call_descriptor, Graph* graph, - SourcePositionTable* source_positions, Code::Kind kind, + JSGraph* jsgraph, SourcePositionTable* source_positions, Code::Kind kind, const char* debug_name, int32_t builtin_index, PoisoningMitigationLevel poisoning_level, const AssemblerOptions& options) { OptimizedCompilationInfo info(CStrVector(debug_name), graph->zone(), kind); @@ -2583,7 +2592,7 @@ MaybeHandle Pipeline::GenerateCodeForCodeStub( bool should_optimize_jumps = isolate->serializer_enabled() && FLAG_turbo_rewrite_far_jumps; PipelineData data(&zone_stats, &info, isolate, isolate->allocator(), graph, - nullptr, source_positions, &node_origins, + jsgraph, nullptr, source_positions, &node_origins, should_optimize_jumps ? &jump_opt : nullptr, options); data.set_verify_graph(FLAG_verify_csa); std::unique_ptr pipeline_statistics; @@ -2633,7 +2642,7 @@ MaybeHandle Pipeline::GenerateCodeForCodeStub( // repeat it for jump optimization. The first run has to happen on a temporary // pipeline to avoid deletion of zones on the main pipeline. PipelineData second_data(&zone_stats, &info, isolate, isolate->allocator(), - data.graph(), data.schedule(), + data.graph(), data.jsgraph(), data.schedule(), data.source_positions(), data.node_origins(), data.jump_optimization_info(), options); second_data.set_verify_graph(FLAG_verify_csa); @@ -2802,7 +2811,8 @@ MaybeHandle Pipeline::GenerateCodeForTesting( ZoneStats zone_stats(isolate->allocator()); NodeOriginTable* node_positions = new (info->zone()) NodeOriginTable(graph); PipelineData data(&zone_stats, info, isolate, isolate->allocator(), graph, - schedule, nullptr, node_positions, nullptr, options); + nullptr, schedule, nullptr, node_positions, nullptr, + options); std::unique_ptr pipeline_statistics; if (FLAG_turbo_stats || FLAG_turbo_stats_nvp) { pipeline_statistics.reset(new PipelineStatistics( diff --git a/src/compiler/pipeline.h b/src/compiler/pipeline.h index 42f31472a9..1e91fcd475 100644 --- a/src/compiler/pipeline.h +++ b/src/compiler/pipeline.h @@ -34,6 +34,7 @@ namespace compiler { class CallDescriptor; class Graph; class InstructionSequence; +class JSGraph; class JSHeapBroker; class MachineGraph; class NodeOriginTable; @@ -72,7 +73,7 @@ class Pipeline : public AllStatic { // Run the pipeline on a machine graph and generate code. static MaybeHandle GenerateCodeForCodeStub( Isolate* isolate, CallDescriptor* call_descriptor, Graph* graph, - SourcePositionTable* source_positions, Code::Kind kind, + JSGraph* jsgraph, SourcePositionTable* source_positions, Code::Kind kind, const char* debug_name, int32_t builtin_index, PoisoningMitigationLevel poisoning_level, const AssemblerOptions& options);