Commit 947a0437 authored by franzih's avatar franzih Committed by Commit bot

Collect type profile for DevTools

Collect type information for JavaScript variables and display it
in Chrome DevTools.
Design Doc: https://docs.google.com/a/google.com/document/d/1O1uepXZXBI6IwiawTrYC3ohhiNgzkyTdjn3R8ysbYgk/edit?usp=sharing

When debugging JavaScript, it’s helpful to know the type of
a variable, parameter, and return values. JavaScript is
dynamically typed, and for complex
source code it’s often hard to infer types. With type profiling, we
can provide type information to JavaScript developers.

This CL is a proof of concept. It collects type profile for
assignments and simply prints the types to stdout.

The output looks something like this:

#my_var1
  #Object
  #number
  #string
  #number
  #undefined
  #string
  #Object
  #Object

We use an extra slot in the feedback vector of assignments to
carry the list of types for that assignment. The extra slot is
only added when the flag --type-profile is given.

Missing work:
* Collect data for parameters and return values (currently only assignments).
* Remove duplicates from the list of collected types and use a common base class.
* Add line numbers or source position instead of the variable name.

For now, has a test that compares the stdout of --type-profile in test/message. We
will remove this test when --type-profile is fully integrated in
the debugger protocol. Adding
the test in test/inspector does not work, because the inspector
test itself consists of JavaScript code that would convolute the
output and be non-deterministic under stress.

BUG=v8:5935

Review-Url: https://codereview.chromium.org/2707873002
Cr-Commit-Position: refs/heads/master@{#43866}
parent a4c73fa7
......@@ -15,7 +15,8 @@ namespace internal {
class AstNumberingVisitor final : public AstVisitor<AstNumberingVisitor> {
public:
AstNumberingVisitor(uintptr_t stack_limit, Zone* zone,
Compiler::EagerInnerFunctionLiterals* eager_literals)
Compiler::EagerInnerFunctionLiterals* eager_literals,
bool collect_type_profile = false)
: zone_(zone),
eager_literals_(eager_literals),
next_id_(BailoutId::FirstUsable().ToInt()),
......@@ -25,7 +26,8 @@ class AstNumberingVisitor final : public AstVisitor<AstNumberingVisitor> {
slot_cache_(zone),
disable_crankshaft_reason_(kNoReason),
dont_optimize_reason_(kNoReason),
catch_prediction_(HandlerTable::UNCAUGHT) {
catch_prediction_(HandlerTable::UNCAUGHT),
collect_type_profile_(collect_type_profile) {
InitializeAstVisitor(stack_limit);
}
......@@ -101,6 +103,7 @@ class AstNumberingVisitor final : public AstVisitor<AstNumberingVisitor> {
BailoutReason disable_crankshaft_reason_;
BailoutReason dont_optimize_reason_;
HandlerTable::CatchPrediction catch_prediction_;
bool collect_type_profile_;
DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
DISALLOW_COPY_AND_ASSIGN(AstNumberingVisitor);
......@@ -422,7 +425,8 @@ void AstNumberingVisitor::VisitAssignment(Assignment* node) {
if (node->is_compound()) VisitBinaryOperation(node->binary_operation());
VisitReference(node->target());
Visit(node->value());
ReserveFeedbackSlots(node);
node->AssignFeedbackSlots(properties_.get_spec(), language_mode_,
&slot_cache_, collect_type_profile_);
}
......@@ -714,12 +718,14 @@ bool AstNumberingVisitor::Renumber(FunctionLiteral* node) {
bool AstNumbering::Renumber(
uintptr_t stack_limit, Zone* zone, FunctionLiteral* function,
Compiler::EagerInnerFunctionLiterals* eager_literals) {
Compiler::EagerInnerFunctionLiterals* eager_literals,
bool collect_type_profile) {
DisallowHeapAllocation no_allocation;
DisallowHandleAllocation no_handles;
DisallowHandleDereference no_deref;
AstNumberingVisitor visitor(stack_limit, zone, eager_literals);
AstNumberingVisitor visitor(stack_limit, zone, eager_literals,
collect_type_profile);
return visitor.Renumber(function);
}
} // namespace internal
......
......@@ -27,7 +27,8 @@ namespace AstNumbering {
// non-null, adds any eager inner literal functions into it.
bool Renumber(
uintptr_t stack_limit, Zone* zone, FunctionLiteral* function,
ThreadedList<ThreadedListZoneEntry<FunctionLiteral*>>* eager_literals);
ThreadedList<ThreadedListZoneEntry<FunctionLiteral*>>* eager_literals,
bool collect_type_profile = false);
}
// Some details on yield IDs
......
......@@ -283,8 +283,13 @@ Assignment::Assignment(Token::Value op, Expression* target, Expression* value,
void Assignment::AssignFeedbackSlots(FeedbackVectorSpec* spec,
LanguageMode language_mode,
FeedbackSlotCache* cache) {
FeedbackSlotCache* cache,
bool collect_type_profile) {
AssignVectorSlots(target(), spec, language_mode, &slot_);
if (collect_type_profile) {
type_profile_slot_ = spec->AddTypeProfileSlot();
}
}
void CountOperation::AssignFeedbackSlots(FeedbackVectorSpec* spec,
......
......@@ -2414,9 +2414,17 @@ class Assignment final : public Expression {
}
void AssignFeedbackSlots(FeedbackVectorSpec* spec, LanguageMode language_mode,
FeedbackSlotCache* cache);
FeedbackSlotCache* cache,
bool collect_type_profile = false);
FeedbackSlot AssignmentSlot() const { return slot_; }
FeedbackSlot TypeProfileSlot() const {
DCHECK(HasTypeProfileSlot());
return type_profile_slot_;
}
bool HasTypeProfileSlot() const { return !type_profile_slot_.IsInvalid(); }
private:
friend class AstNodeFactory;
......@@ -2438,6 +2446,8 @@ class Assignment final : public Expression {
Expression* value_;
BinaryOperation* binary_operation_;
SmallMapList receiver_types_;
FeedbackSlot type_profile_slot_;
};
......
......@@ -478,9 +478,24 @@ bool Renumber(ParseInfo* parse_info,
Compiler::EagerInnerFunctionLiterals* eager_literals) {
RuntimeCallTimerScope runtimeTimer(parse_info->isolate(),
&RuntimeCallStats::CompileRenumber);
// CollectTypeProfile uses its own feedback slots. If we have existing
// FeedbackMetadata, we can only collect type profile, if the feedback vector
// has the appropriate slots.
bool collect_type_profile;
if (parse_info->shared_info().is_null() ||
parse_info->shared_info()->feedback_metadata()->length() == 0) {
collect_type_profile = FLAG_type_profile &&
parse_info->script()->type() == Script::TYPE_NORMAL;
} else {
collect_type_profile =
parse_info->shared_info()->feedback_metadata()->HasTypeProfileSlot();
}
if (!AstNumbering::Renumber(
parse_info->isolate()->stack_guard()->real_climit(),
parse_info->zone(), parse_info->literal(), eager_literals)) {
parse_info->zone(), parse_info->literal(), eager_literals,
collect_type_profile)) {
return false;
}
if (!parse_info->shared_info().is_null()) {
......
......@@ -803,6 +803,22 @@ void BytecodeGraphBuilder::VisitStaDataPropertyInLiteral() {
environment()->RecordAfterState(node, Environment::kAttachFrameState);
}
void BytecodeGraphBuilder::VisitCollectTypeProfile() {
PrepareEagerCheckpoint();
Node* name =
environment()->LookupRegister(bytecode_iterator().GetRegisterOperand(0));
Node* value = environment()->LookupAccumulator();
Node* index = jsgraph()->Constant(bytecode_iterator().GetIndexOperand(1));
Node* vector = jsgraph()->Constant(feedback_vector());
const Operator* op = javascript()->CallRuntime(Runtime::kCollectTypeProfile);
Node* node = NewNode(op, name, value, vector, index);
environment()->RecordAfterState(node, Environment::kAttachFrameState);
}
void BytecodeGraphBuilder::VisitLdaContextSlot() {
const Operator* op = javascript()->LoadContext(
bytecode_iterator().GetUnsignedImmediateOperand(2),
......
......@@ -54,6 +54,7 @@ int FeedbackMetadata::GetSlotSize(FeedbackSlotKind kind) {
case FeedbackSlotKind::kToBoolean:
case FeedbackSlotKind::kLiteral:
case FeedbackSlotKind::kCreateClosure:
case FeedbackSlotKind::kTypeProfile:
return 1;
case FeedbackSlotKind::kCall:
......@@ -186,7 +187,8 @@ void FeedbackVector::ComputeCounts(int* with_type_info, int* generic,
case FeedbackSlotKind::kStoreOwnNamed:
case FeedbackSlotKind::kStoreKeyedSloppy:
case FeedbackSlotKind::kStoreKeyedStrict:
case FeedbackSlotKind::kStoreDataPropertyInLiteral: {
case FeedbackSlotKind::kStoreDataPropertyInLiteral:
case FeedbackSlotKind::kTypeProfile: {
if (obj->IsWeakCell() || obj->IsFixedArray() || obj->IsString()) {
with++;
} else if (obj == megamorphic_sentinel) {
......
......@@ -149,6 +149,8 @@ const char* FeedbackMetadata::Kind2String(FeedbackSlotKind kind) {
return "kCreateClosure";
case FeedbackSlotKind::kLiteral:
return "LITERAL";
case FeedbackSlotKind::kTypeProfile:
return "TYPE_PROFILE";
case FeedbackSlotKind::kGeneral:
return "STUB";
case FeedbackSlotKind::kKindsNumber:
......@@ -158,6 +160,18 @@ const char* FeedbackMetadata::Kind2String(FeedbackSlotKind kind) {
return "?";
}
bool FeedbackMetadata::HasTypeProfileSlot() {
FeedbackMetadataIterator iter(this);
while (iter.HasNext()) {
iter.Next();
FeedbackSlotKind kind = iter.kind();
if (kind == FeedbackSlotKind::kTypeProfile) {
return true;
}
}
return false;
}
FeedbackSlotKind FeedbackVector::GetKind(FeedbackSlot slot) const {
DCHECK(!is_empty());
return metadata()->GetKind(slot);
......@@ -219,6 +233,7 @@ Handle<FeedbackVector> FeedbackVector::New(Isolate* isolate,
case FeedbackSlotKind::kStoreKeyedStrict:
case FeedbackSlotKind::kStoreDataPropertyInLiteral:
case FeedbackSlotKind::kGeneral:
case FeedbackSlotKind::kTypeProfile:
array->set(index, *uninitialized_sentinel, SKIP_WRITE_BARRIER);
break;
......@@ -336,7 +351,8 @@ void FeedbackVector::ClearSlots(JSFunction* host_function) {
break;
}
case FeedbackSlotKind::kCreateClosure: {
break;
case FeedbackSlotKind::kTypeProfile:
break;
}
case FeedbackSlotKind::kGeneral: {
if (obj->IsHeapObject()) {
......@@ -1023,5 +1039,49 @@ void StoreDataPropertyInLiteralICNexus::ConfigureMonomorphic(
SetFeedbackExtra(*name);
}
InlineCacheState CollectTypeProfileNexus::StateFromFeedback() const {
Isolate* isolate = GetIsolate();
Object* const feedback = GetFeedback();
if (feedback == *FeedbackVector::UninitializedSentinel(isolate)) {
return UNINITIALIZED;
}
return MONOMORPHIC;
}
void CollectTypeProfileNexus::Collect(Handle<Name> type) {
Isolate* isolate = GetIsolate();
Object* const feedback = GetFeedback();
Handle<ArrayList> types;
if (feedback == *FeedbackVector::UninitializedSentinel(isolate)) {
types = ArrayList::New(isolate, 1);
} else {
types = Handle<ArrayList>(ArrayList::cast(feedback), isolate);
}
// TODO(franzih): Somehow sort this list. Either avoid duplicates
// or use the common base type.
SetFeedback(*ArrayList::Add(types, type));
}
void CollectTypeProfileNexus::Print() const {
Isolate* isolate = GetIsolate();
Object* const feedback = GetFeedback();
if (feedback == *FeedbackVector::UninitializedSentinel(isolate)) {
return;
}
Handle<ArrayList> list;
list = Handle<ArrayList>(ArrayList::cast(feedback), isolate);
for (int i = 0; i < list->Length(); i++) {
String* name = String::cast(list->Get(i));
PrintF("%s\n", name->ToCString().get());
}
}
} // namespace internal
} // namespace v8
......@@ -36,6 +36,7 @@ enum class FeedbackSlotKind {
kCompareOp,
kToBoolean,
kStoreDataPropertyInLiteral,
kTypeProfile,
kCreateClosure,
kLiteral,
// This is a general purpose slot that occupies one feedback vector element.
......@@ -150,6 +151,11 @@ class FeedbackVectorSpecBase {
return AddSlot(FeedbackSlotKind::kStoreDataPropertyInLiteral);
}
FeedbackSlot AddTypeProfileSlot() {
DCHECK(FLAG_type_profile);
return AddSlot(FeedbackSlotKind::kTypeProfile);
}
#ifdef OBJECT_PRINT
// For gdb debugging.
void Print();
......@@ -250,6 +256,8 @@ class FeedbackMetadata : public FixedArray {
static const char* Kind2String(FeedbackSlotKind kind);
bool HasTypeProfileSlot();
private:
static const int kFeedbackSlotKindBits = 5;
STATIC_ASSERT(static_cast<int>(FeedbackSlotKind::kKindsNumber) <
......@@ -742,6 +750,30 @@ class StoreDataPropertyInLiteralICNexus : public FeedbackNexus {
InlineCacheState StateFromFeedback() const override;
};
// For each assignment, store the type of the value in the collection of types
// in the feedback vector.
class CollectTypeProfileNexus : public FeedbackNexus {
public:
CollectTypeProfileNexus(Handle<FeedbackVector> vector, FeedbackSlot slot)
: FeedbackNexus(vector, slot) {
DCHECK_EQ(FeedbackSlotKind::kTypeProfile, vector->GetKind(slot));
}
CollectTypeProfileNexus(FeedbackVector* vector, FeedbackSlot slot)
: FeedbackNexus(vector, slot) {
DCHECK_EQ(FeedbackSlotKind::kTypeProfile, vector->GetKind(slot));
}
// Add a type to the list of types.
void Collect(Handle<Name> type);
// Dump the types to stdout.
// TODO(franzih): pass this information to the debugger protocol instead of
// stdout.
void Print() const;
InlineCacheState StateFromFeedback() const override;
};
inline BinaryOperationHint BinaryOperationHintFromFeedback(int type_feedback);
inline CompareOperationHint CompareOperationHintFromFeedback(int type_feedback);
......
......@@ -574,6 +574,7 @@ void IC::ConfigureVectorState(Handle<Name> name, Handle<Map> map,
case FeedbackSlotKind::kLiteral:
case FeedbackSlotKind::kGeneral:
case FeedbackSlotKind::kStoreDataPropertyInLiteral:
case FeedbackSlotKind::kTypeProfile:
case FeedbackSlotKind::kInvalid:
case FeedbackSlotKind::kKindsNumber:
UNREACHABLE();
......@@ -620,6 +621,7 @@ void IC::ConfigureVectorState(Handle<Name> name, MapHandleList* maps,
case FeedbackSlotKind::kLiteral:
case FeedbackSlotKind::kGeneral:
case FeedbackSlotKind::kStoreDataPropertyInLiteral:
case FeedbackSlotKind::kTypeProfile:
case FeedbackSlotKind::kInvalid:
case FeedbackSlotKind::kKindsNumber:
UNREACHABLE();
......
......@@ -639,6 +639,13 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::StoreDataPropertyInLiteral(
return *this;
}
BytecodeArrayBuilder& BytecodeArrayBuilder::CollectTypeProfile(
Register name, int feedback_slot) {
DCHECK(FLAG_type_profile);
OutputCollectTypeProfile(name, feedback_slot);
return *this;
}
BytecodeArrayBuilder& BytecodeArrayBuilder::StoreNamedProperty(
Register object, size_t name_index, int feedback_slot,
LanguageMode language_mode) {
......
......@@ -138,6 +138,11 @@ class V8_EXPORT_PRIVATE BytecodeArrayBuilder final
Register object, Register name, DataPropertyInLiteralFlags flags,
int feedback_slot);
// Collect type information for developer tools. The value for which we
// record the type is stored in the accumulator.
// TODO(franzih): Do not pass the name, instead use the source position.
BytecodeArrayBuilder& CollectTypeProfile(Register name, int feedback_slot);
// Store a property named by a property name. The value to be stored should be
// in the accumulator.
BytecodeArrayBuilder& StoreNamedProperty(Register object,
......
......@@ -2211,18 +2211,34 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
LhsKind assign_type = Property::GetAssignType(property);
// Evaluate LHS expression.
Register lhs_name;
if (expr->HasTypeProfileSlot()) {
lhs_name = register_allocator()->NewRegister();
}
switch (assign_type) {
case VARIABLE:
if (expr->HasTypeProfileSlot()) {
builder()
->LoadLiteral(expr->target()->AsVariableProxy()->var()->raw_name())
.StoreAccumulatorInRegister(lhs_name);
}
// Nothing to do to evaluate variable assignment LHS.
break;
case NAMED_PROPERTY: {
object = VisitForRegisterValue(property->obj());
name = property->key()->AsLiteral()->AsRawPropertyName();
if (expr->HasTypeProfileSlot()) {
builder()->LoadLiteral(name).StoreAccumulatorInRegister(lhs_name);
}
break;
}
case KEYED_PROPERTY: {
object = VisitForRegisterValue(property->obj());
key = VisitForRegisterValue(property->key());
if (expr->HasTypeProfileSlot()) {
builder()->StoreAccumulatorInRegister(lhs_name);
}
break;
}
case NAMED_SUPER_PROPERTY: {
......@@ -2235,6 +2251,9 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
builder()
->LoadLiteral(property->key()->AsLiteral()->AsRawPropertyName())
.StoreAccumulatorInRegister(super_property_args[2]);
if (expr->HasTypeProfileSlot()) {
builder()->StoreAccumulatorInRegister(lhs_name);
}
break;
}
case KEYED_SUPER_PROPERTY: {
......@@ -2245,6 +2264,10 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
VisitForRegisterValue(super_property->home_object(),
super_property_args[1]);
VisitForRegisterValue(property->key(), super_property_args[2]);
if (expr->HasTypeProfileSlot()) {
builder()->StoreAccumulatorInRegister(lhs_name);
}
break;
}
}
......@@ -2333,6 +2356,14 @@ void BytecodeGenerator::VisitAssignment(Assignment* expr) {
break;
}
}
// Value is in accumulator.
if (expr->HasTypeProfileSlot()) {
FeedbackSlot collect_type_feedback_slot = expr->TypeProfileSlot();
builder()->CollectTypeProfile(lhs_name,
feedback_index(collect_type_feedback_slot));
}
}
void BytecodeGenerator::VisitYield(Yield* expr) {
......
......@@ -105,6 +105,8 @@ namespace interpreter {
OperandType::kReg, OperandType::kIdx) \
V(StaDataPropertyInLiteral, AccumulatorUse::kRead, OperandType::kReg, \
OperandType::kReg, OperandType::kFlag8, OperandType::kIdx) \
V(CollectTypeProfile, AccumulatorUse::kRead, OperandType::kReg, \
OperandType::kIdx) \
\
/* Binary Operators */ \
V(Add, AccumulatorUse::kReadWrite, OperandType::kReg, OperandType::kIdx) \
......
......@@ -1045,6 +1045,19 @@ void Interpreter::DoStaDataPropertyInLiteral(InterpreterAssembler* assembler) {
__ Dispatch();
}
void Interpreter::DoCollectTypeProfile(InterpreterAssembler* assembler) {
Node* name = __ LoadRegister(__ BytecodeOperandReg(0));
Node* value = __ GetAccumulator();
Node* vector_index = __ SmiTag(__ BytecodeOperandIdx(1));
Node* feedback_vector = __ LoadFeedbackVector();
Node* context = __ GetContext();
__ CallRuntime(Runtime::kCollectTypeProfile, context, name, value,
feedback_vector, vector_index);
__ Dispatch();
}
// LdaModuleVariable <cell_index> <depth>
//
// Load the contents of a module variable into the accumulator. The variable is
......
......@@ -751,6 +751,7 @@ void FeedbackVector::FeedbackVectorPrint(std::ostream& os) { // NOLINT
case FeedbackSlotKind::kCreateClosure:
case FeedbackSlotKind::kLiteral:
case FeedbackSlotKind::kGeneral:
case FeedbackSlotKind::kTypeProfile:
break;
case FeedbackSlotKind::kToBoolean:
case FeedbackSlotKind::kInvalid:
......
......@@ -691,6 +691,32 @@ RUNTIME_FUNCTION(Runtime_DefineDataPropertyInLiteral) {
return *object;
}
RUNTIME_FUNCTION(Runtime_CollectTypeProfile) {
HandleScope scope(isolate);
DCHECK_EQ(4, args.length());
CONVERT_ARG_HANDLE_CHECKED(String, name, 0);
CONVERT_ARG_HANDLE_CHECKED(Object, value, 1);
CONVERT_ARG_HANDLE_CHECKED(FeedbackVector, vector, 2);
CONVERT_SMI_ARG_CHECKED(index, 3);
DCHECK(FLAG_type_profile);
Handle<Name> type = Object::TypeOf(isolate, value);
if (value->IsJSReceiver()) {
Handle<JSReceiver> object = Handle<JSReceiver>::cast(value);
type = JSReceiver::GetConstructorName(object);
}
CollectTypeProfileNexus nexus(vector, vector->ToSlot(index));
nexus.Collect(type);
PrintF("%s\n", name->ToCString().get());
nexus.Print();
PrintF("\n");
return *name;
}
// Return property without being observable by accessors or interceptors.
RUNTIME_FUNCTION(Runtime_GetDataProperty) {
HandleScope scope(isolate);
......
......@@ -394,6 +394,7 @@ namespace internal {
F(IsJSGlobalProxy, 1, 1) \
F(DefineAccessorPropertyUnchecked, 5, 1) \
F(DefineDataPropertyInLiteral, 6, 1) \
F(CollectTypeProfile, 4, 1) \
F(GetDataProperty, 2, 1) \
F(GetConstructorName, 1, 1) \
F(HasFastPackedElements, 1, 1) \
......
// Copyright 2016 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --type-profile --turbo
function test(param) {
var my_var1 = param;
var my_var2 = 17;
}
test({});
test(123);
test('hello');
test(123);
test(undefined);
test('hello');
test({x: 12});
test({x: 12});
class MyClass {
constructor() {}
}
function testConstructorNames(param) {
var my_var = param;
}
testConstructorNames(new MyClass());
testConstructorNames({});
testConstructorNames(2);
throw "throw otherwise test fails with --stress-opt";
my_var1
Object
my_var2
number
my_var1
Object
number
my_var2
number
number
my_var1
Object
number
string
my_var2
number
number
number
my_var1
Object
number
string
number
my_var2
number
number
number
number
my_var1
Object
number
string
number
undefined
my_var2
number
number
number
number
number
my_var1
Object
number
string
number
undefined
string
my_var2
number
number
number
number
number
number
my_var1
Object
number
string
number
undefined
string
Object
my_var2
number
number
number
number
number
number
number
my_var1
Object
number
string
number
undefined
string
Object
Object
my_var2
number
number
number
number
number
number
number
number
Function
MyClass
Function
my_var
MyClass
my_var
MyClass
Object
my_var
MyClass
Object
number
*%(basename)s:34: throw otherwise test fails with --stress-opt
throw "throw otherwise test fails with --stress-opt";
^
......@@ -447,6 +447,12 @@ TEST_F(BytecodeArrayBuilderTest, AllBytecodesGenerated) {
scorecard[Bytecodes::ToByte(Bytecode::kTestNull)] = 1;
}
if (!FLAG_type_profile) {
// Bytecode for CollectTypeProfile is only emitted when
// Type Information for DevTools is turned on.
scorecard[Bytecodes::ToByte(Bytecode::kCollectTypeProfile)] = 1;
}
// Check return occurs at the end and only once in the BytecodeArray.
CHECK_EQ(final_bytecode, Bytecode::kReturn);
CHECK_EQ(scorecard[Bytecodes::ToByte(final_bytecode)], 1);
......
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