Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
P
Pyston
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
0
Merge Requests
0
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
Boxiang Sun
Pyston
Commits
82a63d19
Commit
82a63d19
authored
Feb 24, 2015
by
Kevin Modzelewski
Browse files
Options
Browse Files
Download
Plain Diff
Merge pull request #325 from undingen/llvm_rebase
Rebase llvm to r230300
parents
fb9159ca
ef1acbdc
Changes
13
Hide whitespace changes
Inline
Side-by-side
Showing
13 changed files
with
95 additions
and
49 deletions
+95
-49
CMakeLists.txt
CMakeLists.txt
+1
-1
Makefile
Makefile
+1
-1
llvm_patches/0008-Add-llvm.experimental.patchpoint.double.patch
...atches/0008-Add-llvm.experimental.patchpoint.double.patch
+37
-37
llvm_revision.txt
llvm_revision.txt
+1
-1
src/codegen/irgen.cpp
src/codegen/irgen.cpp
+12
-1
src/codegen/irgen/irgenerator.cpp
src/codegen/irgen/irgenerator.cpp
+2
-2
src/codegen/opt/const_classes.cpp
src/codegen/opt/const_classes.cpp
+3
-1
src/codegen/opt/inliner.cpp
src/codegen/opt/inliner.cpp
+12
-1
src/codegen/runtime_hooks.cpp
src/codegen/runtime_hooks.cpp
+4
-0
src/codegen/unwinding.cpp
src/codegen/unwinding.cpp
+4
-0
src/runtime/dict.cpp
src/runtime/dict.cpp
+1
-1
src/runtime/stacktrace.cpp
src/runtime/stacktrace.cpp
+0
-2
tools/publicize.cpp
tools/publicize.cpp
+17
-1
No files found.
CMakeLists.txt
View file @
82a63d19
...
...
@@ -91,7 +91,7 @@ endif()
add_subdirectory
(
${
DEPS_DIR
}
/llvm-trunk
${
CMAKE_BINARY_DIR
}
/llvm EXCLUDE_FROM_ALL
)
set
(
CMAKE_MODULE_PATH
"
${
CMAKE_BINARY_DIR
}
/llvm/share/llvm/cmake/"
)
include
(
LLVMConfig
)
llvm_map_components_to_libnames
(
LLVM_LIBS core mcjit native bitreader ipo irreader debuginfo instrumentation
${
INTEL_JIT_EVENTS_LIB
}
)
llvm_map_components_to_libnames
(
LLVM_LIBS core mcjit native bitreader ipo irreader debuginfo
dwarf
instrumentation
${
INTEL_JIT_EVENTS_LIB
}
)
# libunwind
if
(
"
${
CMAKE_BUILD_TYPE
}
"
STREQUAL
"Debug"
)
...
...
Makefile
View file @
82a63d19
...
...
@@ -94,7 +94,7 @@ else
LLVM_BIN
:=
$(LLVM_BUILD)
/Release/bin
endif
LLVM_LINK_LIBS
:=
core mcjit native bitreader ipo irreader debuginfo instrumentation
LLVM_LINK_LIBS
:=
core mcjit native bitreader ipo irreader debuginfo
dwarf
instrumentation
ifneq
($(ENABLE_INTEL_JIT_EVENTS),0)
LLVM_LINK_LIBS
+=
inteljitevents
endif
...
...
llvm_patches/0008-Add-llvm.experimental.patchpoint.double.patch
View file @
82a63d19
From
3b888292dab18580ad6f234fd6bc4b92a7f54c46
Mon Sep 17 00:00:00 2001
From
b1fd8c81830c14e712b3c2da5379a506a11dc59f
Mon Sep 17 00:00:00 2001
From: Kevin Modzelewski <kevmod@gmail.com>
Date: Thu, 9 Oct 2014 04:37:33 +0000
Subject: [PATCH] Add llvm.experimental.patchpoint.double
Differential Revision: http://reviews.llvm.org/D5696
---
docs/StackMaps.rst |
3 +++
include/llvm/IR/Intrinsics.td |
5 +++++
lib/CodeGen/SelectionDAG/FastISel.cpp |
8
++++++-
lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp |
4 +++-
lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp |
1 +
lib/IR/Verifier.cpp |
3 ++-
lib/Target/AArch64/AArch64TargetTransformInfo.cpp |
1 +
lib/Target/X86/X86TargetTransformInfo.cpp |
1 +
test/CodeGen/X86/anyregcc.ll |
24
+++++++++++++++++++--
test/CodeGen/X86/patchpoint.ll |
18
++++++++++++++++
10 files changed, 6
3 insertions(+), 5
deletions(-)
docs/StackMaps.rst | 3 +++
include/llvm/IR/Intrinsics.td | 5 +++++
lib/CodeGen/SelectionDAG/FastISel.cpp |
8 +
++++++-
lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp | 4 +++-
lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | 1 +
lib/IR/Verifier.cpp |
1 +
lib/Target/AArch64/AArch64TargetTransformInfo.cpp | 1 +
lib/Target/X86/X86TargetTransformInfo.cpp | 1 +
test/CodeGen/X86/anyregcc.ll |
24 ++
+++++++++++++++++++--
test/CodeGen/X86/patchpoint.ll |
18 +
++++++++++++++++
10 files changed, 6
2 insertions(+), 4
deletions(-)
diff --git a/docs/StackMaps.rst b/docs/StackMaps.rst
index
bd0fb94..5f6d83b
100644
index
43c60c9..c900c97
100644
--- a/docs/StackMaps.rst
+++ b/docs/StackMaps.rst
@@ -192,6 +192,9 @@
Syntax:
...
...
@@ -32,10 +32,10 @@ index bd0fb94..5f6d83b 100644
Overview:
"""""""""
diff --git a/include/llvm/IR/Intrinsics.td b/include/llvm/IR/Intrinsics.td
index
1b9339a..45eecca
100644
index
e89e65d..1603067
100644
--- a/include/llvm/IR/Intrinsics.td
+++ b/include/llvm/IR/Intrinsics.td
@@ -
490,4 +490,9
@@
def int_experimental_patchpoint_i64 : Intrinsic<[llvm_i64_ty],
@@ -
514,6 +514,11
@@
def int_experimental_patchpoint_i64 : Intrinsic<[llvm_i64_ty],
llvm_ptr_ty, llvm_i32_ty,
llvm_vararg_ty],
[Throws]>;
...
...
@@ -45,11 +45,13 @@ index 1b9339a..45eecca 100644
+ llvm_vararg_ty],
+ [Throws]>;
//===------------------------ Garbage Collection Intrinsics ---------------===//
diff --git a/lib/CodeGen/SelectionDAG/FastISel.cpp b/lib/CodeGen/SelectionDAG/FastISel.cpp
index
72390a9..b624a63
100644
index
1df4a1d..54af94b
100644
--- a/lib/CodeGen/SelectionDAG/FastISel.cpp
+++ b/lib/CodeGen/SelectionDAG/FastISel.cpp
@@ -73
8,7 +738
,12 @@
bool FastISel::selectPatchpoint(const CallInst *I) {
@@ -73
9,7 +739
,12 @@
bool FastISel::selectPatchpoint(const CallInst *I) {
// Add an explicit result reg if we use the anyreg calling convention.
if (IsAnyRegCC && HasDef) {
assert(CLI.NumResultRegs == 0 && "Unexpected result register.");
...
...
@@ -63,7 +65,7 @@ index 72390a9..b624a63 100644
CLI.NumResultRegs = 1;
Ops.push_back(MachineOperand::CreateReg(CLI.ResultReg, /*IsDef=*/true));
}
@@ -120
5,6 +1210
,7 @@
bool FastISel::selectIntrinsicCall(const IntrinsicInst *II) {
@@ -120
6,6 +1211
,7 @@
bool FastISel::selectIntrinsicCall(const IntrinsicInst *II) {
return selectStackmap(II);
case Intrinsic::experimental_patchpoint_void:
case Intrinsic::experimental_patchpoint_i64:
...
...
@@ -72,10 +74,10 @@ index 72390a9..b624a63 100644
}
diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index
a63c90a..dc45bf7
100644
index
29c0360..c0a8299
100644
--- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -202
6,6 +2026
,7 @@
void SelectionDAGBuilder::visitInvoke(const InvokeInst &I) {
@@ -202
5,6 +2025
,7 @@
void SelectionDAGBuilder::visitInvoke(const InvokeInst &I) {
break;
case Intrinsic::experimental_patchpoint_void:
case Intrinsic::experimental_patchpoint_i64:
...
...
@@ -83,7 +85,7 @@ index a63c90a..dc45bf7 100644
visitPatchpoint(&I, LandingPad);
break;
}
@@ -55
36,7 +553
7,8 @@
SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I, unsigned Intrinsic) {
@@ -55
96,7 +559
7,8 @@
SelectionDAGBuilder::visitIntrinsicCall(const CallInst &I, unsigned Intrinsic) {
return nullptr;
}
case Intrinsic::experimental_patchpoint_void:
...
...
@@ -94,10 +96,10 @@ index a63c90a..dc45bf7 100644
return nullptr;
}
diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index
31f4b06..ac0e63
5 100644
index
5e867cf..cb3f99
5 100644
--- a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -1
014,6 +1014
,7 @@
static void collectFailStats(const Instruction *I) {
@@ -1
101,6 +1101
,7 @@
static void collectFailStats(const Instruction *I) {
NumFastIselFailStackMap++; return;
case Intrinsic::experimental_patchpoint_void: // fall-through
case Intrinsic::experimental_patchpoint_i64:
...
...
@@ -106,42 +108,40 @@ index 31f4b06..ac0e635 100644
}
}
diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp
index
5519cac..cec9229
100644
index
d01e138..be969eb
100644
--- a/lib/IR/Verifier.cpp
+++ b/lib/IR/Verifier.cpp
@@ -2218,7 +2218,8 @@
void Verifier::visitInstruction(Instruction &I) {
Assert1(!F->isIntrinsic() || isa<CallInst>(I) ||
@@ -2487,6 +2487,7 @@
void Verifier::visitInstruction(Instruction &I) {
F->getIntrinsicID() == Intrinsic::donothing ||
F->getIntrinsicID() == Intrinsic::experimental_patchpoint_void ||
- F->getIntrinsicID() == Intrinsic::experimental_patchpoint_i64,
+ F->getIntrinsicID() == Intrinsic::experimental_patchpoint_
i64
||
+ F->getIntrinsicID() == Intrinsic::experimental_patchpoint_double
,
F->getIntrinsicID() == Intrinsic::experimental_patchpoint_i64 ||
+ F->getIntrinsicID() == Intrinsic::experimental_patchpoint_
double
||
F->getIntrinsicID() == Intrinsic::experimental_gc_statepoint
,
"Cannot invoke an intrinsinc other than"
" donothing or patchpoint", &I);
Assert1(F->getParent() == M, "Referencing function in another module!",
diff --git a/lib/Target/AArch64/AArch64TargetTransformInfo.cpp b/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
index
dbdf199..17ea7d0
100644
index
0646d85..40f2c85
100644
--- a/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
+++ b/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
@@ -
280,6 +280,7 @@
unsigned AArch64TTI
::getIntImmCost(Intrinsic::ID IID, unsigned Idx,
@@ -
159,6 +159,7 @@
unsigned AArch64TTIImpl
::getIntImmCost(Intrinsic::ID IID, unsigned Idx,
break;
case Intrinsic::experimental_patchpoint_void:
case Intrinsic::experimental_patchpoint_i64:
+ case Intrinsic::experimental_patchpoint_double:
if ((Idx < 4) || (Imm.getBitWidth() <= 64 && isInt<64>(Imm.getSExtValue())))
return TCC_Free;
return T
TI::T
CC_Free;
break;
diff --git a/lib/Target/X86/X86TargetTransformInfo.cpp b/lib/Target/X86/X86TargetTransformInfo.cpp
index 5
31e035..c274edf
100644
index 5
136619..9f455eb
100644
--- a/lib/Target/X86/X86TargetTransformInfo.cpp
+++ b/lib/Target/X86/X86TargetTransformInfo.cpp
@@ -11
41,6 +1141,7 @@
unsigned X86TTI
::getIntImmCost(Intrinsic::ID IID, unsigned Idx,
@@ -11
02,6 +1102,7 @@
unsigned X86TTIImpl
::getIntImmCost(Intrinsic::ID IID, unsigned Idx,
break;
case Intrinsic::experimental_patchpoint_void:
case Intrinsic::experimental_patchpoint_i64:
+ case Intrinsic::experimental_patchpoint_double:
if ((Idx < 4) || (Imm.getBitWidth() <= 64 && isInt<64>(Imm.getSExtValue())))
return TCC_Free;
return T
TI::T
CC_Free;
break;
diff --git a/test/CodeGen/X86/anyregcc.ll b/test/CodeGen/X86/anyregcc.ll
index 98ba17c..b81a3ad 100644
...
...
@@ -233,5 +233,5 @@ index 07148f0..7e2a340 100644
declare i64 @llvm.experimental.patchpoint.i64(i64, i32, i8*, i32, ...)
+declare double @llvm.experimental.patchpoint.double(i64, i32, i8*, i32, ...)
--
1.7.9.5
2.1.0
llvm_revision.txt
View file @
82a63d19
2
250
00
2
303
00
src/codegen/irgen.cpp
View file @
82a63d19
...
...
@@ -23,7 +23,11 @@
#include "llvm/IR/DIBuilder.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/Verifier.h"
#if LLVMREV < 229094
#include "llvm/PassManager.h"
#else
#include "llvm/IR/LegacyPassManager.h"
#endif
#include "llvm/Support/FileSystem.h"
#include "llvm/Target/TargetMachine.h"
#include "llvm/Target/TargetSubtargetInfo.h"
...
...
@@ -71,7 +75,12 @@ static void optimizeIR(llvm::Function* f, EffortLevel effort) {
Timer
_t
(
"optimizing"
);
#if LLVMREV < 229094
llvm
::
FunctionPassManager
fpm
(
g
.
cur_module
);
#else
llvm
::
legacy
::
FunctionPassManager
fpm
(
g
.
cur_module
);
#endif
#if LLVMREV < 217548
fpm
.
add
(
new
llvm
::
DataLayoutPass
(
*
g
.
tm
->
getDataLayout
()));
...
...
@@ -898,8 +907,10 @@ CompiledFunction* doCompile(SourceInfo* source, ParamNames* param_names, const O
g
.
cur_module
=
new
llvm
::
Module
(
name
,
g
.
context
);
#if LLVMREV < 217070 // not sure if this is the right rev
g
.
cur_module
->
setDataLayout
(
g
.
tm
->
getDataLayout
()
->
getStringRepresentation
());
#el
se
#el
if LLVMREV < 227113
g
.
cur_module
->
setDataLayout
(
g
.
tm
->
getSubtargetImpl
()
->
getDataLayout
());
#else
g
.
cur_module
->
setDataLayout
(
g
.
tm
->
getDataLayout
());
#endif
// g.engine->addModule(g.cur_module);
...
...
src/codegen/irgen/irgenerator.cpp
View file @
82a63d19
...
...
@@ -501,7 +501,7 @@ private:
ConcreteCompilerVariable
*
converted_obj
=
obj
->
makeConverted
(
emitter
,
obj
->
getBoxType
());
obj
->
decvref
(
emitter
);
llvm
::
Value
*
v
=
emitter
.
createCall
(
unw_info
,
g
.
funcs
.
getPystonIter
,
{
converted_obj
->
getValue
()
}
);
llvm
::
Value
*
v
=
emitter
.
createCall
(
unw_info
,
g
.
funcs
.
getPystonIter
,
converted_obj
->
getValue
()
);
assert
(
v
->
getType
()
==
g
.
llvm_value_type_ptr
);
return
new
ConcreteCompilerVariable
(
UNKNOWN
,
v
,
true
);
...
...
@@ -1647,7 +1647,7 @@ private:
// end code for handling of softspace
llvm
::
Value
*
v
=
emitter
.
createCall
(
unw_info
,
g
.
funcs
.
str
,
{
converted
->
getValue
()
}
);
llvm
::
Value
*
v
=
emitter
.
createCall
(
unw_info
,
g
.
funcs
.
str
,
converted
->
getValue
()
);
v
=
emitter
.
getBuilder
()
->
CreateBitCast
(
v
,
g
.
llvm_value_type_ptr
);
auto
s
=
new
ConcreteCompilerVariable
(
STR
,
v
,
true
);
r
=
dest
->
callattr
(
emitter
,
getOpInfoForNode
(
node
,
unw_info
),
&
write_str
,
false
,
ArgPassSpec
(
1
),
{
s
},
...
...
src/codegen/opt/const_classes.cpp
View file @
82a63d19
...
...
@@ -99,8 +99,10 @@ private:
APInt
ap_offset
(
64
,
0
,
true
);
#if LLVMREV < 214781
bool
success
=
gep
->
accumulateConstantOffset
(
*
g
.
tm
->
getDataLayout
(),
ap_offset
);
#el
se
#el
if LLVMREV < 227113
bool
success
=
gep
->
accumulateConstantOffset
(
*
g
.
tm
->
getSubtargetImpl
()
->
getDataLayout
(),
ap_offset
);
#else
bool
success
=
gep
->
accumulateConstantOffset
(
*
g
.
tm
->
getDataLayout
(),
ap_offset
);
#endif
assert
(
success
);
int64_t
offset
=
ap_offset
.
getSExtValue
();
...
...
src/codegen/opt/inliner.cpp
View file @
82a63d19
...
...
@@ -22,7 +22,11 @@
#include "llvm/IR/IntrinsicInst.h"
#include "llvm/IR/Module.h"
#include "llvm/IR/Verifier.h"
#if LLVMREV < 229094
#include "llvm/PassManager.h"
#else
#include "llvm/IR/LegacyPassManager.h"
#endif
#include "llvm/Support/raw_ostream.h"
#include "llvm/Transforms/IPO.h"
#include "llvm/Transforms/IPO/PassManagerBuilder.h"
...
...
@@ -99,8 +103,11 @@ public:
if
(
!
initialized
)
{
llvm
::
initializeInlineCostAnalysisPass
(
*
llvm
::
PassRegistry
::
getPassRegistry
());
llvm
::
initializeSimpleInlinerPass
(
*
llvm
::
PassRegistry
::
getPassRegistry
());
#if LLVMREV < 227669
llvm
::
initializeTargetTransformInfoAnalysisGroup
(
*
llvm
::
PassRegistry
::
getPassRegistry
());
#else
llvm
::
initializeTargetTransformInfoWrapperPassPass
(
*
llvm
::
PassRegistry
::
getPassRegistry
());
#endif
fake_module
=
new
llvm
::
Module
(
"fake"
,
g
.
context
);
initialized
=
true
;
...
...
@@ -119,7 +126,11 @@ public:
llvm
::
Module
*
cur_module
=
f
.
getParent
();
#if LLVMREV < 217548
llvm
::
PassManager
fake_pm
;
#else
llvm
::
legacy
::
PassManager
fake_pm
;
#endif
llvm
::
InlineCostAnalysis
*
cost_analysis
=
new
llvm
::
InlineCostAnalysis
();
fake_pm
.
add
(
cost_analysis
);
// llvm::errs() << "doing fake run\n";
...
...
src/codegen/runtime_hooks.cpp
View file @
82a63d19
...
...
@@ -20,7 +20,11 @@
#include "llvm/Analysis/Passes.h"
#include "llvm/ExecutionEngine/ExecutionEngine.h"
#include "llvm/IR/Module.h"
#if LLVMREV < 229094
#include "llvm/PassManager.h"
#else
#include "llvm/IR/LegacyPassManager.h"
#endif
#include "llvm/Support/DynamicLibrary.h"
#include "llvm/Support/TargetSelect.h"
#include "llvm/Transforms/Scalar.h"
...
...
src/codegen/unwinding.cpp
View file @
82a63d19
...
...
@@ -18,7 +18,11 @@
#include <sys/types.h>
#include <unistd.h>
#if LLVMREV < 227586
#include "llvm/DebugInfo/DIContext.h"
#else
#include "llvm/DebugInfo/DWARF/DIContext.h"
#endif
#include "llvm/ExecutionEngine/JITEventListener.h"
#include "llvm/IR/DebugInfo.h"
#include "llvm/Object/ObjectFile.h"
...
...
src/runtime/dict.cpp
View file @
82a63d19
...
...
@@ -257,7 +257,7 @@ extern "C" int PyDict_Next(PyObject* op, Py_ssize_t* ppos, PyObject** pkey, PyOb
// Clients are supposed to zero-initialize *ppos:
if
(
*
it_ptr
==
NULL
)
{
*
it_ptr
=
(
iterator
*
)
malloc
(
sizeof
(
iterator
));
**
it_ptr
=
self
->
d
.
begin
();
**
it_ptr
=
self
->
d
.
begin
();
}
iterator
*
it
=
*
it_ptr
;
...
...
src/runtime/stacktrace.cpp
View file @
82a63d19
...
...
@@ -16,8 +16,6 @@
#include <cstdarg>
#include <dlfcn.h>
#include "llvm/DebugInfo/DIContext.h"
#include "codegen/unwinding.h"
#include "core/options.h"
#include "gc/collector.h"
...
...
tools/publicize.cpp
View file @
82a63d19
...
...
@@ -89,7 +89,23 @@ bool makeVisible(llvm::GlobalValue* gv) {
if
(
visibility
==
llvm
::
GlobalValue
::
HiddenVisibility
)
{
gv
->
setVisibility
(
llvm
::
GlobalValue
::
ProtectedVisibility
);
//gv->setDLLStorageClass(llvm::GlobalValue::DLLExportStorageClass);
gv
->
setName
(
gv
->
getName
()
+
"_protected"
);
std
::
string
old_name
=
gv
->
getName
();
std
::
string
new_name
=
(
gv
->
getName
()
+
"_protected"
).
str
();
gv
->
setName
(
new_name
);
#if LLVMREV >= 225705
// If this symbol has comdat set we have to remove the comdat for the old name
// and create a new comdat with the new name
if
(
gv
->
hasComdat
()
&&
isa
<
llvm
::
GlobalObject
>
(
gv
))
{
llvm
::
Module
*
mod
=
gv
->
getParent
();
llvm
::
GlobalObject
*
go
=
cast
<
llvm
::
GlobalObject
>
(
gv
);
llvm
::
Comdat
*
new_com
=
mod
->
getOrInsertComdat
(
new_name
);
new_com
->
setSelectionKind
(
go
->
getComdat
()
->
getSelectionKind
());
go
->
setComdat
(
new_com
);
auto
&
Comdats
=
mod
->
getComdatSymbolTable
();
Comdats
.
erase
(
Comdats
.
find
(
old_name
));
}
#endif
changed
=
true
;
}
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment