From 858602d8d9cab6beb98dfae9b3827259afc6f2f1 Mon Sep 17 00:00:00 2001 From: Leszek Swirski Date: Mon, 26 Sep 2022 13:22:29 +0200 Subject: [PATCH] [turbofan] Add dynamic sized GraphAssemblerLabels The GraphAssemblerLabel VarCount template parameter now can have a marker value ~0 which is marker for it being dynamic sized -- this means that a bit of template magic turns its std::arrays into std::vectors. Merging GraphAssemblerLabels works by duck-typing access to these arrays/vectors. These dynamic GraphAssemblerLabels are created whenever a single GraphAssemblerLabels being created when instead a list of values convertible to MachineRepresentation is passed in. Passing anything else will result in a GraphAssemblerLabel with marker value ~1, which is considered "invalid" and will give a compilation error down the line. std: :vector is passed into MakeLabel, with the static Change-Id: I833bdedac2f8e26fcc88aa59dd67b7e4b1c4296d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3913349 Commit-Queue: Leszek Swirski Reviewed-by: Tobias Tebbi Cr-Commit-Position: refs/heads/main@{#83424} --- src/compiler/graph-assembler.h | 198 ++++++++++++++++++++++---------- src/compiler/js-call-reducer.cc | 5 +- 2 files changed, 138 insertions(+), 65 deletions(-) diff --git a/src/compiler/graph-assembler.h b/src/compiler/graph-assembler.h index 6dd3e3d7cf..92fd273060 100644 --- a/src/compiler/graph-assembler.h +++ b/src/compiler/graph-assembler.h @@ -5,6 +5,9 @@ #ifndef V8_COMPILER_GRAPH_ASSEMBLER_H_ #define V8_COMPILER_GRAPH_ASSEMBLER_H_ +#include + +#include "src/base/small-vector.h" #include "src/codegen/tnode.h" #include "src/compiler/feedback-source.h" #include "src/compiler/js-graph.h" @@ -150,10 +153,44 @@ class GraphAssembler; enum class GraphAssemblerLabelType { kDeferred, kNonDeferred, kLoop }; +namespace detail { +constexpr size_t kGraphAssemblerLabelDynamicCount = ~0u; + +template +struct GraphAssemblerHelper { + template + using Array = std::array; + static constexpr bool kIsDynamic = false; + + static Array InitNodeArray(const Array& reps) { + return {}; + } +}; +template <> +struct GraphAssemblerHelper { + // TODO(leszeks): We could allow other sizes of small vector here, by encoding + // the size in the negative VarCount. + template + using Array = base::SmallVector; + static constexpr bool kIsDynamic = true; + + static Array InitNodeArray(const Array& reps) { + return Array(reps.size()); + } +}; +} // namespace detail + // Label with statically known count of incoming branches and phis. template class GraphAssemblerLabel { + using Helper = detail::GraphAssemblerHelper; + template + using Array = typename Helper::template Array; + static constexpr bool kIsDynamic = Helper::kIsDynamic; + public: + size_t Count() { return representations_.size(); } + Node* PhiAt(size_t index); template @@ -166,10 +203,11 @@ class GraphAssemblerLabel { bool IsUsed() const { return merged_count_ > 0; } GraphAssemblerLabel(GraphAssemblerLabelType type, int loop_nesting_level, - const std::array& reps) + Array reps) : type_(type), loop_nesting_level_(loop_nesting_level), - representations_(reps) {} + bindings_(Helper::InitNodeArray(reps)), + representations_(std::move(reps)) {} ~GraphAssemblerLabel() { DCHECK(IsBound() || merged_count_ == 0); } @@ -192,10 +230,43 @@ class GraphAssemblerLabel { size_t merged_count_ = 0; Node* effect_; Node* control_; - std::array bindings_; - const std::array representations_; + Array bindings_; + const Array representations_; }; +using GraphAssemblerDynamicLabel = + GraphAssemblerLabel; + +namespace detail { +template +struct GraphAssemblerLabelForXHelper; + +// If the Us are a template pack each assignable to T, use a static label. +template +struct GraphAssemblerLabelForXHelper< + T, std::enable_if_t...>>, + Us...> { + using Type = GraphAssemblerLabel; +}; + +// If the single arg is a vector of U assignable to T, use a dynamic label. +template +struct GraphAssemblerLabelForXHelper< + T, std::enable_if_t>, base::SmallVector> { + using Type = GraphAssemblerDynamicLabel; +}; + +template +using GraphAssemblerLabelForVars = + typename GraphAssemblerLabelForXHelper::Type; + +template +using GraphAssemblerLabelForReps = + typename GraphAssemblerLabelForXHelper::Type; + +} // namespace detail + using NodeChangedCallback = std::function; class V8_EXPORT_PRIVATE GraphAssembler { public: @@ -212,35 +283,34 @@ class V8_EXPORT_PRIVATE GraphAssembler { // Create label. template - GraphAssemblerLabel MakeLabelFor( + detail::GraphAssemblerLabelForReps MakeLabelFor( GraphAssemblerLabelType type, Reps... reps) { std::array reps_array = {reps...}; - return MakeLabel(reps_array, type); + return detail::GraphAssemblerLabelForReps( + type, loop_nesting_level_, std::move(reps_array)); } - - // As above, but with an std::array of machine representations. - template - GraphAssemblerLabel MakeLabel( - std::array reps_array, - GraphAssemblerLabelType type) { - return GraphAssemblerLabel(type, loop_nesting_level_, reps_array); + GraphAssemblerDynamicLabel MakeLabelFor( + GraphAssemblerLabelType type, + base::SmallVector reps) { + return GraphAssemblerDynamicLabel(type, loop_nesting_level_, + std::move(reps)); } // Convenience wrapper for creating non-deferred labels. template - GraphAssemblerLabel MakeLabel(Reps... reps) { + detail::GraphAssemblerLabelForReps MakeLabel(Reps... reps) { return MakeLabelFor(GraphAssemblerLabelType::kNonDeferred, reps...); } // Convenience wrapper for creating loop labels. template - GraphAssemblerLabel MakeLoopLabel(Reps... reps) { + detail::GraphAssemblerLabelForReps MakeLoopLabel(Reps... reps) { return MakeLabelFor(GraphAssemblerLabelType::kLoop, reps...); } // Convenience wrapper for creating deferred labels. template - GraphAssemblerLabel MakeDeferredLabel(Reps... reps) { + detail::GraphAssemblerLabelForReps MakeDeferredLabel(Reps... reps) { return MakeLabelFor(GraphAssemblerLabelType::kDeferred, reps...); } @@ -349,7 +419,7 @@ class V8_EXPORT_PRIVATE GraphAssembler { void Bind(GraphAssemblerLabel* label); template - void Goto(GraphAssemblerLabel* label, Vars...); + void Goto(detail::GraphAssemblerLabelForVars* label, Vars...); // Branch hints are inferred from if_true/if_false deferred states. void BranchWithCriticalSafetyCheck(Node* condition, @@ -358,13 +428,14 @@ class V8_EXPORT_PRIVATE GraphAssembler { // Branch hints are inferred from if_true/if_false deferred states. template - void Branch(Node* condition, GraphAssemblerLabel* if_true, - GraphAssemblerLabel* if_false, Vars...); + void Branch(Node* condition, + detail::GraphAssemblerLabelForVars* if_true, + detail::GraphAssemblerLabelForVars* if_false, Vars...); template void BranchWithHint(Node* condition, - GraphAssemblerLabel* if_true, - GraphAssemblerLabel* if_false, + detail::GraphAssemblerLabelForVars* if_true, + detail::GraphAssemblerLabelForVars* if_false, BranchHint hint, Vars...); // Control helpers. @@ -372,7 +443,8 @@ class V8_EXPORT_PRIVATE GraphAssembler { // {GotoIf(c, l, h)} is equivalent to {BranchWithHint(c, l, templ, h); // Bind(templ)}. template - void GotoIf(Node* condition, GraphAssemblerLabel* label, + void GotoIf(Node* condition, + detail::GraphAssemblerLabelForVars* label, BranchHint hint, Vars...); // {GotoIfNot(c, l, h)} is equivalent to {BranchWithHint(c, templ, l, h); @@ -381,18 +453,19 @@ class V8_EXPORT_PRIVATE GraphAssembler { // so {GotoIfNot(..., BranchHint::kTrue)} means "optimize for the case where // the branch is *not* taken". template - void GotoIfNot(Node* condition, GraphAssemblerLabel* label, + void GotoIfNot(Node* condition, + detail::GraphAssemblerLabelForVars* label, BranchHint hint, Vars...); // {GotoIf(c, l)} is equivalent to {Branch(c, l, templ);Bind(templ)}. template - void GotoIf(Node* condition, GraphAssemblerLabel* label, - Vars...); + void GotoIf(Node* condition, + detail::GraphAssemblerLabelForVars* label, Vars...); // {GotoIfNot(c, l)} is equivalent to {Branch(c, templ, l);Bind(templ)}. template - void GotoIfNot(Node* condition, GraphAssemblerLabel* label, - Vars...); + void GotoIfNot(Node* condition, + detail::GraphAssemblerLabelForVars* label, Vars...); bool HasActiveBlock() const { // This is false if the current block has been terminated (e.g. by a Goto or @@ -437,7 +510,8 @@ class V8_EXPORT_PRIVATE GraphAssembler { protected: template - void MergeState(GraphAssemblerLabel* label, Vars... vars); + void MergeState(detail::GraphAssemblerLabelForVars* label, + Vars... vars); V8_INLINE Node* AddClonedNode(Node* node); @@ -525,8 +599,8 @@ class V8_EXPORT_PRIVATE GraphAssembler { template void BranchImpl(Node* condition, - GraphAssemblerLabel* if_true, - GraphAssemblerLabel* if_false, + detail::GraphAssemblerLabelForVars* if_true, + detail::GraphAssemblerLabelForVars* if_false, BranchHint hint, Vars...); Zone* temp_zone_; @@ -556,18 +630,21 @@ class V8_EXPORT_PRIVATE GraphAssembler { template Node* GraphAssemblerLabel::PhiAt(size_t index) { DCHECK(IsBound()); - DCHECK_LT(index, VarCount); + DCHECK_LT(index, Count()); return bindings_[index]; } template -void GraphAssembler::MergeState(GraphAssemblerLabel* label, - Vars... vars) { +void GraphAssembler::MergeState( + detail::GraphAssemblerLabelForVars* label, Vars... vars) { + using NodeArray = typename detail::GraphAssemblerLabelForVars< + Vars...>::template Array; RestoreEffectControlScope restore_effect_control_scope(this); const int merged_count = static_cast(label->merged_count_); - static constexpr int kVarCount = sizeof...(vars); - std::array var_array = {vars...}; + + const size_t var_count = label->Count(); + NodeArray var_array{vars...}; const bool is_loop_exit = label->loop_nesting_level_ != loop_nesting_level_; if (is_loop_exit) { @@ -585,7 +662,7 @@ void GraphAssembler::MergeState(GraphAssemblerLabel* label, AddNode(graph()->NewNode(common()->LoopExit(), control(), *loop_headers_.back())); AddNode(graph()->NewNode(common()->LoopExitEffect(), effect(), control())); - for (size_t i = 0; i < kVarCount; i++) { + for (size_t i = 0; i < var_count; i++) { var_array[i] = AddNode(graph()->NewNode( common()->LoopExitValue(MachineRepresentation::kTagged), var_array[i], control())); @@ -602,7 +679,7 @@ void GraphAssembler::MergeState(GraphAssemblerLabel* label, Node* terminate = graph()->NewNode(common()->Terminate(), label->effect_, label->control_); NodeProperties::MergeControlToEnd(graph(), common(), terminate); - for (size_t i = 0; i < kVarCount; i++) { + for (size_t i = 0; i < var_count; i++) { label->bindings_[i] = graph()->NewNode(common()->Phi(label->representations_[i], 2), var_array[i], var_array[i], label->control_); @@ -612,7 +689,7 @@ void GraphAssembler::MergeState(GraphAssemblerLabel* label, DCHECK_EQ(1, merged_count); label->control_->ReplaceInput(1, control()); label->effect_->ReplaceInput(1, effect()); - for (size_t i = 0; i < kVarCount; i++) { + for (size_t i = 0; i < var_count; i++) { label->bindings_[i]->ReplaceInput(1, var_array[i]); CHECK(!NodeProperties::IsTyped(var_array[i])); // Unsupported. } @@ -624,7 +701,7 @@ void GraphAssembler::MergeState(GraphAssemblerLabel* label, // Just set the control, effect and variables directly. label->control_ = control(); label->effect_ = effect(); - for (size_t i = 0; i < kVarCount; i++) { + for (size_t i = 0; i < var_count; i++) { label->bindings_[i] = var_array[i]; } } else if (merged_count == 1) { @@ -633,7 +710,7 @@ void GraphAssembler::MergeState(GraphAssemblerLabel* label, graph()->NewNode(common()->Merge(2), label->control_, control()); label->effect_ = graph()->NewNode(common()->EffectPhi(2), label->effect_, effect(), label->control_); - for (size_t i = 0; i < kVarCount; i++) { + for (size_t i = 0; i < var_count; i++) { label->bindings_[i] = graph()->NewNode( common()->Phi(label->representations_[i], 2), label->bindings_[i], var_array[i], label->control_); @@ -651,7 +728,7 @@ void GraphAssembler::MergeState(GraphAssemblerLabel* label, NodeProperties::ChangeOp(label->effect_, common()->EffectPhi(merged_count + 1)); - for (size_t i = 0; i < kVarCount; i++) { + for (size_t i = 0; i < var_count; i++) { DCHECK_EQ(IrOpcode::kPhi, label->bindings_[i]->opcode()); label->bindings_[i]->ReplaceInput(merged_count, var_array[i]); label->bindings_[i]->AppendInput(graph()->zone(), label->control_); @@ -686,7 +763,7 @@ void GraphAssembler::Bind(GraphAssemblerLabel* label) { if (label->merged_count_ > 1 || label->IsLoop()) { AddNode(label->control_); AddNode(label->effect_); - for (size_t i = 0; i < VarCount; i++) { + for (size_t i = 0; i < label->Count(); i++) { AddNode(label->bindings_[i]); } } else { @@ -697,10 +774,9 @@ void GraphAssembler::Bind(GraphAssemblerLabel* label) { } template -void GraphAssembler::Branch(Node* condition, - GraphAssemblerLabel* if_true, - GraphAssemblerLabel* if_false, - Vars... vars) { +void GraphAssembler::Branch( + Node* condition, detail::GraphAssemblerLabelForVars* if_true, + detail::GraphAssemblerLabelForVars* if_false, Vars... vars) { BranchHint hint = BranchHint::kNone; if (if_true->IsDeferred() != if_false->IsDeferred()) { hint = if_false->IsDeferred() ? BranchHint::kTrue : BranchHint::kFalse; @@ -711,17 +787,17 @@ void GraphAssembler::Branch(Node* condition, template void GraphAssembler::BranchWithHint( - Node* condition, GraphAssemblerLabel* if_true, - GraphAssemblerLabel* if_false, BranchHint hint, + Node* condition, detail::GraphAssemblerLabelForVars* if_true, + detail::GraphAssemblerLabelForVars* if_false, BranchHint hint, Vars... vars) { BranchImpl(condition, if_true, if_false, hint, vars...); } template -void GraphAssembler::BranchImpl(Node* condition, - GraphAssemblerLabel* if_true, - GraphAssemblerLabel* if_false, - BranchHint hint, Vars... vars) { +void GraphAssembler::BranchImpl( + Node* condition, detail::GraphAssemblerLabelForVars* if_true, + detail::GraphAssemblerLabelForVars* if_false, BranchHint hint, + Vars... vars) { DCHECK_NOT_NULL(control()); Node* branch = graph()->NewNode(common()->Branch(hint), condition, control()); @@ -737,7 +813,7 @@ void GraphAssembler::BranchImpl(Node* condition, } template -void GraphAssembler::Goto(GraphAssemblerLabel* label, +void GraphAssembler::Goto(detail::GraphAssemblerLabelForVars* label, Vars... vars) { DCHECK_NOT_NULL(control()); DCHECK_NOT_NULL(effect()); @@ -749,7 +825,7 @@ void GraphAssembler::Goto(GraphAssemblerLabel* label, template void GraphAssembler::GotoIf(Node* condition, - GraphAssemblerLabel* label, + detail::GraphAssemblerLabelForVars* label, BranchHint hint, Vars... vars) { Node* branch = graph()->NewNode(common()->Branch(hint), condition, control()); @@ -760,9 +836,9 @@ void GraphAssembler::GotoIf(Node* condition, } template -void GraphAssembler::GotoIfNot(Node* condition, - GraphAssemblerLabel* label, - BranchHint hint, Vars... vars) { +void GraphAssembler::GotoIfNot( + Node* condition, detail::GraphAssemblerLabelForVars* label, + BranchHint hint, Vars... vars) { Node* branch = graph()->NewNode(common()->Branch(hint), condition, control()); control_ = graph()->NewNode(common()->IfFalse(), branch); @@ -773,7 +849,7 @@ void GraphAssembler::GotoIfNot(Node* condition, template void GraphAssembler::GotoIf(Node* condition, - GraphAssemblerLabel* label, + detail::GraphAssemblerLabelForVars* label, Vars... vars) { BranchHint hint = label->IsDeferred() ? BranchHint::kFalse : BranchHint::kNone; @@ -781,9 +857,9 @@ void GraphAssembler::GotoIf(Node* condition, } template -void GraphAssembler::GotoIfNot(Node* condition, - GraphAssemblerLabel* label, - Vars... vars) { +void GraphAssembler::GotoIfNot( + Node* condition, detail::GraphAssemblerLabelForVars* label, + Vars... vars) { BranchHint hint = label->IsDeferred() ? BranchHint::kTrue : BranchHint::kNone; return GotoIfNot(condition, label, hint, vars...); } diff --git a/src/compiler/js-call-reducer.cc b/src/compiler/js-call-reducer.cc index da53d19f98..0c11c1abe7 100644 --- a/src/compiler/js-call-reducer.cc +++ b/src/compiler/js-call-reducer.cc @@ -744,10 +744,7 @@ class IteratingArrayBuiltinReducerAssembler : public JSCallReducerAssembler { TNode... vars) { if (!IsHoleyElementsKind(kind)) return o; - std::array reps = { - MachineRepresentationOf::value...}; - auto if_not_hole = - MakeLabel(reps, GraphAssemblerLabelType::kNonDeferred); + auto if_not_hole = MakeLabel(MachineRepresentationOf::value...); BranchWithHint(HoleCheck(kind, o), continue_label, &if_not_hole, BranchHint::kFalse, vars...);