Skip to content

Commit d91e8d8

Browse files
schuayCommit Bot
authored andcommitted
[compiler] Hook in unary op builtins with feedback in generic lowering
If --turbo-nci is enabled, use unary op builtins with feedback collection during generic lowering. Bug: v8:8888 Change-Id: Ie32cfe1558a7fbada2ac69a99ef969097558bc89 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2209067 Commit-Queue: Jakob Gruber <jgruber@chromium.org> Reviewed-by: Georg Neis <neis@chromium.org> Cr-Commit-Position: refs/heads/master@{#67962}
1 parent c8e3cbb commit d91e8d8

6 files changed

Lines changed: 113 additions & 49 deletions

File tree

src/compiler/bytecode-graph-builder.cc

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2810,19 +2810,27 @@ SpeculationMode BytecodeGraphBuilder::GetSpeculationMode(int slot_id) const {
28102810
}
28112811

28122812
void BytecodeGraphBuilder::VisitBitwiseNot() {
2813-
BuildUnaryOp(javascript()->BitwiseNot());
2813+
FeedbackSource feedback = CreateFeedbackSource(
2814+
bytecode_iterator().GetSlotOperand(kUnaryOperationHintIndex).ToInt());
2815+
BuildUnaryOp(javascript()->BitwiseNot(feedback));
28142816
}
28152817

28162818
void BytecodeGraphBuilder::VisitDec() {
2817-
BuildUnaryOp(javascript()->Decrement());
2819+
FeedbackSource feedback = CreateFeedbackSource(
2820+
bytecode_iterator().GetSlotOperand(kUnaryOperationHintIndex).ToInt());
2821+
BuildUnaryOp(javascript()->Decrement(feedback));
28182822
}
28192823

28202824
void BytecodeGraphBuilder::VisitInc() {
2821-
BuildUnaryOp(javascript()->Increment());
2825+
FeedbackSource feedback = CreateFeedbackSource(
2826+
bytecode_iterator().GetSlotOperand(kUnaryOperationHintIndex).ToInt());
2827+
BuildUnaryOp(javascript()->Increment(feedback));
28222828
}
28232829

28242830
void BytecodeGraphBuilder::VisitNegate() {
2825-
BuildUnaryOp(javascript()->Negate());
2831+
FeedbackSource feedback = CreateFeedbackSource(
2832+
bytecode_iterator().GetSlotOperand(kUnaryOperationHintIndex).ToInt());
2833+
BuildUnaryOp(javascript()->Negate(feedback));
28262834
}
28272835

28282836
void BytecodeGraphBuilder::VisitAdd() {

src/compiler/globals.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,22 @@
66
#define V8_COMPILER_GLOBALS_H_
77

88
#include "src/common/globals.h"
9+
#include "src/flags/flags.h"
910

1011
namespace v8 {
1112
namespace internal {
1213
namespace compiler {
1314

15+
// The nci flag is currently used to experiment with feedback collection in
16+
// optimized code produced by generic lowering.
17+
// Considerations:
18+
// - Should we increment the call count? https://crbug.com/v8/10524
19+
// - Is feedback already megamorphic in all these cases?
20+
//
21+
// TODO(jgruber): Remove once we've made a decision whether to collect feedback
22+
// unconditionally.
23+
inline bool CollectFeedbackInGenericLowering() { return FLAG_turbo_nci; }
24+
1425
enum class StackCheckKind {
1526
kJSFunctionEntry = 0,
1627
kJSIterationBody,

src/compiler/js-generic-lowering.cc

Lines changed: 60 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,6 @@ namespace compiler {
2525

2626
namespace {
2727

28-
// The nci flag is currently used to experiment with feedback collection in
29-
// optimized code produced by generic lowering.
30-
// Considerations:
31-
// - Should we increment the call count? https://crbug.com/v8/10524
32-
// - Is feedback already megamorphic in all these cases?
33-
//
34-
// TODO(jgruber): Remove once we've made a decision whether to collect feedback
35-
// unconditionally.
36-
bool CollectFeedback() { return FLAG_turbo_nci; }
37-
3828
CallDescriptor::Flags FrameStateFlagForCall(Node* node) {
3929
return OperatorProperties::HasFrameStateInput(node->op())
4030
? CallDescriptor::kNeedsFrameState
@@ -87,10 +77,6 @@ REPLACE_STUB_CALL(LessThan)
8777
REPLACE_STUB_CALL(LessThanOrEqual)
8878
REPLACE_STUB_CALL(GreaterThan)
8979
REPLACE_STUB_CALL(GreaterThanOrEqual)
90-
REPLACE_STUB_CALL(BitwiseNot)
91-
REPLACE_STUB_CALL(Decrement)
92-
REPLACE_STUB_CALL(Increment)
93-
REPLACE_STUB_CALL(Negate)
9480
REPLACE_STUB_CALL(HasProperty)
9581
REPLACE_STUB_CALL(Equal)
9682
REPLACE_STUB_CALL(ToLength)
@@ -111,6 +97,8 @@ REPLACE_STUB_CALL(RejectPromise)
11197
REPLACE_STUB_CALL(ResolvePromise)
11298
#undef REPLACE_STUB_CALL
11399

100+
// TODO(jgruber): Hook in binary and compare op builtins with feedback.
101+
114102
void JSGenericLowering::ReplaceWithStubCall(Node* node,
115103
Callable callable,
116104
CallDescriptor::Flags flags) {
@@ -157,6 +145,42 @@ void JSGenericLowering::LowerJSStrictEqual(Node* node) {
157145
Operator::kEliminatable);
158146
}
159147

148+
void JSGenericLowering::ReplaceUnaryOpWithBuiltinCall(
149+
Node* node, Builtins::Name builtin_without_feedback,
150+
Builtins::Name builtin_with_feedback) {
151+
const FeedbackParameter& p = FeedbackParameterOf(node->op());
152+
CallDescriptor::Flags flags = FrameStateFlagForCall(node);
153+
if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
154+
Callable callable = Builtins::CallableFor(isolate(), builtin_with_feedback);
155+
Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
156+
Node* slot = jsgraph()->Int32Constant(p.feedback().slot.ToInt());
157+
const CallInterfaceDescriptor& descriptor = callable.descriptor();
158+
auto call_descriptor = Linkage::GetStubCallDescriptor(
159+
zone(), descriptor, descriptor.GetStackParameterCount(), flags,
160+
node->op()->properties());
161+
Node* stub_code = jsgraph()->HeapConstant(callable.code());
162+
node->InsertInput(zone(), 0, stub_code);
163+
node->InsertInput(zone(), 2, slot);
164+
node->InsertInput(zone(), 3, feedback_vector);
165+
NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
166+
} else {
167+
Callable callable =
168+
Builtins::CallableFor(isolate(), builtin_without_feedback);
169+
ReplaceWithStubCall(node, callable, flags);
170+
}
171+
}
172+
173+
#define DEF_UNARY_LOWERING(Name) \
174+
void JSGenericLowering::LowerJS##Name(Node* node) { \
175+
ReplaceUnaryOpWithBuiltinCall(node, Builtins::k##Name, \
176+
Builtins::k##Name##_WithFeedback); \
177+
}
178+
DEF_UNARY_LOWERING(BitwiseNot)
179+
DEF_UNARY_LOWERING(Decrement)
180+
DEF_UNARY_LOWERING(Increment)
181+
DEF_UNARY_LOWERING(Negate)
182+
#undef DEF_UNARY_LOWERING
183+
160184
namespace {
161185
bool ShouldUseMegamorphicLoadBuiltin(FeedbackSource const& source,
162186
JSHeapBroker* broker) {
@@ -709,26 +733,26 @@ void JSGenericLowering::LowerJSConstruct(Node* node) {
709733
static constexpr int kReceiver = 1;
710734
static constexpr int kMaybeFeedbackVector = 1;
711735

712-
if (CollectFeedback() && p.feedback().IsValid()) {
736+
if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
713737
const int stack_argument_count =
714738
arg_count + kReceiver + kMaybeFeedbackVector;
715739
Callable callable =
716740
Builtins::CallableFor(isolate(), Builtins::kConstruct_WithFeedback);
717741
auto call_descriptor = Linkage::GetStubCallDescriptor(
718742
zone(), callable.descriptor(), stack_argument_count, flags);
719-
Node* maybe_feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
743+
Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
720744
Node* stub_code = jsgraph()->HeapConstant(callable.code());
721745
Node* new_target = node->InputAt(arg_count + 1);
722746
Node* stub_arity = jsgraph()->Int32Constant(arg_count);
723747
Node* slot = jsgraph()->Int32Constant(p.feedback().index());
724748
Node* receiver = jsgraph()->UndefinedConstant();
725749
node->RemoveInput(arg_count + 1); // Drop new target.
726750
// Register argument inputs are followed by stack argument inputs (such as
727-
// maybe_feedback_vector). Both are listed in ascending order. Note that
751+
// feedback_vector). Both are listed in ascending order. Note that
728752
// the receiver is implicitly placed on the stack and is thus inserted
729753
// between explicitly-specified register and stack arguments.
730754
// TODO(jgruber): Implement a simpler way to specify these mutations.
731-
node->InsertInput(zone(), arg_count + 1, maybe_feedback_vector);
755+
node->InsertInput(zone(), arg_count + 1, feedback_vector);
732756
node->InsertInput(zone(), 0, stub_code);
733757
node->InsertInput(zone(), 2, new_target);
734758
node->InsertInput(zone(), 3, stub_arity);
@@ -763,7 +787,7 @@ void JSGenericLowering::LowerJSConstructWithArrayLike(Node* node) {
763787
static constexpr int kArgumentList = 1;
764788
static constexpr int kMaybeFeedbackVector = 1;
765789

766-
if (CollectFeedback() && p.feedback().IsValid()) {
790+
if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
767791
const int stack_argument_count =
768792
arg_count - kArgumentList + kReceiver + kMaybeFeedbackVector;
769793
Callable callable = Builtins::CallableFor(
@@ -774,20 +798,20 @@ void JSGenericLowering::LowerJSConstructWithArrayLike(Node* node) {
774798
Node* receiver = jsgraph()->UndefinedConstant();
775799
Node* arguments_list = node->InputAt(1);
776800
Node* new_target = node->InputAt(2);
777-
Node* maybe_feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
801+
Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
778802
Node* slot = jsgraph()->Int32Constant(p.feedback().index());
779803

780804
node->InsertInput(zone(), 0, stub_code);
781805
node->ReplaceInput(2, new_target);
782806
node->ReplaceInput(3, arguments_list);
783807
// Register argument inputs are followed by stack argument inputs (such as
784-
// maybe_feedback_vector). Both are listed in ascending order. Note that
808+
// feedback_vector). Both are listed in ascending order. Note that
785809
// the receiver is implicitly placed on the stack and is thus inserted
786810
// between explicitly-specified register and stack arguments.
787811
// TODO(jgruber): Implement a simpler way to specify these mutations.
788812
node->InsertInput(zone(), 4, slot);
789813
node->InsertInput(zone(), 5, receiver);
790-
node->InsertInput(zone(), 6, maybe_feedback_vector);
814+
node->InsertInput(zone(), 6, feedback_vector);
791815

792816
NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
793817
} else {
@@ -819,15 +843,15 @@ void JSGenericLowering::LowerJSConstructWithSpread(Node* node) {
819843
static constexpr int kTheSpread = 1; // Included in `arg_count`.
820844
static constexpr int kMaybeFeedbackVector = 1;
821845

822-
if (CollectFeedback() && p.feedback().IsValid()) {
846+
if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
823847
const int stack_argument_count =
824848
arg_count + kReceiver + kMaybeFeedbackVector;
825849
Callable callable = Builtins::CallableFor(
826850
isolate(), Builtins::kConstructWithSpread_WithFeedback);
827851
auto call_descriptor = Linkage::GetStubCallDescriptor(
828852
zone(), callable.descriptor(), stack_argument_count, flags);
829853
Node* stub_code = jsgraph()->HeapConstant(callable.code());
830-
Node* maybe_feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
854+
Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
831855
Node* slot = jsgraph()->Int32Constant(p.feedback().index());
832856

833857
// The single available register is needed for `slot`, thus `spread` remains
@@ -838,11 +862,11 @@ void JSGenericLowering::LowerJSConstructWithSpread(Node* node) {
838862
node->RemoveInput(new_target_index);
839863

840864
// Register argument inputs are followed by stack argument inputs (such as
841-
// maybe_feedback_vector). Both are listed in ascending order. Note that
865+
// feedback_vector). Both are listed in ascending order. Note that
842866
// the receiver is implicitly placed on the stack and is thus inserted
843867
// between explicitly-specified register and stack arguments.
844868
// TODO(jgruber): Implement a simpler way to specify these mutations.
845-
node->InsertInput(zone(), arg_count + 1, maybe_feedback_vector);
869+
node->InsertInput(zone(), arg_count + 1, feedback_vector);
846870
node->InsertInput(zone(), 0, stub_code);
847871
node->InsertInput(zone(), 2, new_target);
848872
node->InsertInput(zone(), 3, stub_arity);
@@ -895,19 +919,19 @@ void JSGenericLowering::LowerJSCall(Node* node) {
895919
int const arg_count = static_cast<int>(p.arity() - 2);
896920
ConvertReceiverMode const mode = p.convert_mode();
897921

898-
if (CollectFeedback() && p.feedback().IsValid()) {
922+
if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
899923
Callable callable = CodeFactory::Call_WithFeedback(isolate(), mode);
900924
CallDescriptor::Flags flags = FrameStateFlagForCall(node);
901925
auto call_descriptor = Linkage::GetStubCallDescriptor(
902926
zone(), callable.descriptor(), arg_count + 1, flags);
903927
Node* stub_code = jsgraph()->HeapConstant(callable.code());
904928
Node* stub_arity = jsgraph()->Int32Constant(arg_count);
905-
Node* maybe_feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
929+
Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
906930
Node* slot = jsgraph()->Int32Constant(p.feedback().index());
907931
node->InsertInput(zone(), 0, stub_code);
908932
node->InsertInput(zone(), 2, stub_arity);
909933
node->InsertInput(zone(), 3, slot);
910-
node->InsertInput(zone(), 4, maybe_feedback_vector);
934+
node->InsertInput(zone(), 4, feedback_vector);
911935
NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
912936
} else {
913937
Callable callable = CodeFactory::Call(isolate(), mode);
@@ -930,21 +954,21 @@ void JSGenericLowering::LowerJSCallWithArrayLike(Node* node) {
930954
DCHECK_EQ(arg_count, 0);
931955
static constexpr int kReceiver = 1;
932956

933-
if (CollectFeedback() && p.feedback().IsValid()) {
957+
if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
934958
Callable callable = Builtins::CallableFor(
935959
isolate(), Builtins::kCallWithArrayLike_WithFeedback);
936960
auto call_descriptor = Linkage::GetStubCallDescriptor(
937961
zone(), callable.descriptor(), arg_count + kReceiver, flags);
938962
Node* stub_code = jsgraph()->HeapConstant(callable.code());
939963
Node* receiver = node->InputAt(1);
940964
Node* arguments_list = node->InputAt(2);
941-
Node* maybe_feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
965+
Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
942966
Node* slot = jsgraph()->Int32Constant(p.feedback().index());
943967
node->InsertInput(zone(), 0, stub_code);
944968
node->ReplaceInput(2, arguments_list);
945969
node->ReplaceInput(3, receiver);
946970
node->InsertInput(zone(), 3, slot);
947-
node->InsertInput(zone(), 4, maybe_feedback_vector);
971+
node->InsertInput(zone(), 4, feedback_vector);
948972
NodeProperties::ChangeOp(node, common()->Call(call_descriptor));
949973
} else {
950974
Callable callable = CodeFactory::CallWithArrayLike(isolate());
@@ -969,14 +993,14 @@ void JSGenericLowering::LowerJSCallWithSpread(Node* node) {
969993
static constexpr int kTheSpread = 1;
970994
static constexpr int kMaybeFeedbackVector = 1;
971995

972-
if (CollectFeedback() && p.feedback().IsValid()) {
996+
if (CollectFeedbackInGenericLowering() && p.feedback().IsValid()) {
973997
const int stack_argument_count = arg_count + kMaybeFeedbackVector;
974998
Callable callable = Builtins::CallableFor(
975999
isolate(), Builtins::kCallWithSpread_WithFeedback);
9761000
auto call_descriptor = Linkage::GetStubCallDescriptor(
9771001
zone(), callable.descriptor(), stack_argument_count, flags);
9781002
Node* stub_code = jsgraph()->HeapConstant(callable.code());
979-
Node* maybe_feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
1003+
Node* feedback_vector = jsgraph()->HeapConstant(p.feedback().vector);
9801004
Node* slot = jsgraph()->Int32Constant(p.feedback().index());
9811005

9821006
// We pass the spread in a register, not on the stack.
@@ -985,11 +1009,11 @@ void JSGenericLowering::LowerJSCallWithSpread(Node* node) {
9851009
node->RemoveInput(spread_index);
9861010

9871011
// Register argument inputs are followed by stack argument inputs (such as
988-
// maybe_feedback_vector). Both are listed in ascending order. Note that
1012+
// feedback_vector). Both are listed in ascending order. Note that
9891013
// the receiver is implicitly placed on the stack and is thus inserted
9901014
// between explicitly-specified register and stack arguments.
9911015
// TODO(jgruber): Implement a simpler way to specify these mutations.
992-
node->InsertInput(zone(), arg_count + 1, maybe_feedback_vector);
1016+
node->InsertInput(zone(), arg_count + 1, feedback_vector);
9931017
node->InsertInput(zone(), 0, stub_code);
9941018
node->InsertInput(zone(), 2, stub_arity);
9951019
node->InsertInput(zone(), 3, spread);

src/compiler/js-generic-lowering.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ class JSGenericLowering final : public AdvancedReducer {
4242
Operator::Properties properties);
4343
void ReplaceWithRuntimeCall(Node* node, Runtime::FunctionId f, int args = -1);
4444

45+
void ReplaceUnaryOpWithBuiltinCall(Node* node,
46+
Builtins::Name builtin_without_feedback,
47+
Builtins::Name builtin_with_feedback);
48+
4549
Zone* zone() const;
4650
Isolate* isolate() const;
4751
JSGraph* jsgraph() const { return jsgraph_; }

src/compiler/js-operator.cc

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,12 @@ std::ostream& operator<<(std::ostream& os, FeedbackParameter const& p) {
226226
}
227227

228228
FeedbackParameter const& FeedbackParameterOf(const Operator* op) {
229-
DCHECK(op->opcode() == IrOpcode::kJSCreateEmptyLiteralArray ||
229+
DCHECK(op->opcode() == IrOpcode::kJSDecrement ||
230+
op->opcode() == IrOpcode::kJSBitwiseNot ||
231+
op->opcode() == IrOpcode::kJSCreateEmptyLiteralArray ||
232+
op->opcode() == IrOpcode::kJSIncrement ||
230233
op->opcode() == IrOpcode::kJSInstanceOf ||
234+
op->opcode() == IrOpcode::kJSNegate ||
231235
op->opcode() == IrOpcode::kJSStoreDataPropertyInLiteral ||
232236
op->opcode() == IrOpcode::kJSStoreInArrayLiteral);
233237
return OpParameter<FeedbackParameter>(op);
@@ -659,10 +663,6 @@ CompareOperationHint CompareOperationHintOf(const Operator* op) {
659663
V(Divide, Operator::kNoProperties, 2, 1) \
660664
V(Modulus, Operator::kNoProperties, 2, 1) \
661665
V(Exponentiate, Operator::kNoProperties, 2, 1) \
662-
V(BitwiseNot, Operator::kNoProperties, 1, 1) \
663-
V(Decrement, Operator::kNoProperties, 1, 1) \
664-
V(Increment, Operator::kNoProperties, 1, 1) \
665-
V(Negate, Operator::kNoProperties, 1, 1) \
666666
V(ToLength, Operator::kNoProperties, 1, 1) \
667667
V(ToName, Operator::kNoProperties, 1, 1) \
668668
V(ToNumber, Operator::kNoProperties, 1, 1) \
@@ -699,6 +699,12 @@ CompareOperationHint CompareOperationHintOf(const Operator* op) {
699699
V(ParseInt, Operator::kNoProperties, 2, 1) \
700700
V(RegExpTest, Operator::kNoProperties, 2, 1)
701701

702+
#define UNARY_OP_LIST(V) \
703+
V(BitwiseNot) \
704+
V(Decrement) \
705+
V(Increment) \
706+
V(Negate)
707+
702708
#define BINARY_OP_LIST(V) V(Add)
703709

704710
#define COMPARE_OP_LIST(V) \
@@ -815,6 +821,16 @@ CACHED_OP_LIST(CACHED_OP)
815821
BINARY_OP_LIST(BINARY_OP)
816822
#undef BINARY_OP
817823

824+
#define UNARY_OP(Name) \
825+
const Operator* JSOperatorBuilder::Name(FeedbackSource const& feedback) { \
826+
FeedbackParameter parameters(feedback); \
827+
return new (zone()) Operator1<FeedbackParameter>( \
828+
IrOpcode::kJS##Name, Operator::kNoProperties, "JS" #Name, 1, 1, 1, 1, \
829+
1, 2, parameters); \
830+
}
831+
UNARY_OP_LIST(UNARY_OP)
832+
#undef UNARY_OP
833+
818834
#define COMPARE_OP(Name, ...) \
819835
const Operator* JSOperatorBuilder::Name(CompareOperationHint hint) { \
820836
switch (hint) { \
@@ -1418,6 +1434,7 @@ Handle<ScopeInfo> ScopeInfoOf(const Operator* op) {
14181434
return OpParameter<Handle<ScopeInfo>>(op);
14191435
}
14201436

1437+
#undef UNARY_OP_LIST
14211438
#undef BINARY_OP_LIST
14221439
#undef CACHED_OP_LIST
14231440
#undef COMPARE_OP_LIST

src/compiler/js-operator.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -771,10 +771,10 @@ class V8_EXPORT_PRIVATE JSOperatorBuilder final
771771
const Operator* Modulus();
772772
const Operator* Exponentiate();
773773

774-
const Operator* BitwiseNot();
775-
const Operator* Decrement();
776-
const Operator* Increment();
777-
const Operator* Negate();
774+
const Operator* BitwiseNot(FeedbackSource const& feedback);
775+
const Operator* Decrement(FeedbackSource const& feedback);
776+
const Operator* Increment(FeedbackSource const& feedback);
777+
const Operator* Negate(FeedbackSource const& feedback);
778778

779779
const Operator* ToLength();
780780
const Operator* ToName();

0 commit comments

Comments
 (0)