* Edited NoHandleAllocation and AssertNoAllocation to be null operations
when
activated on the compiler thread.
* Calling CompilationInfo::AbortOptimization from
OptimizingCompiler::OptimizeGraph from the compiler thread (rightly)
crashes v8
in debug mode. Fixed this.
http://codereview.chromium.org/10807024/diff/2001/src/compiler.cc
File src/compiler.cc (right):
http://codereview.chromium.org/10807024/diff/2001/src/compiler.cc#newcode875
src/compiler.cc:875: // If crankshaft succeeded, install the optimized
code else install
On 2012/07/19 12:47:30, Yang wrote:
"If crankshaft succeeded, install the optimized code. Otherwise
install the
unoptimized code."
Done.
http://codereview.chromium.org/10807024/diff/2001/src/execution.cc
File src/execution.cc (right):
http://codereview.chromium.org/10807024/diff/2001/src/execution.cc#newcode966
src/execution.cc:966:
On 2012/07/19 12:47:30, Yang wrote:
Stray edit?
Done.
http://codereview.chromium.org/10807024/diff/2001/src/heap.cc
File src/heap.cc (right):
http://codereview.chromium.org/10807024/diff/2001/src/heap.cc#newcode6252
src/heap.cc:6252: if (FLAG_parallel_recompilation) delete
relocation_mutex_;
On 2012/07/19 12:47:30, Yang wrote:
I think you can omit checking for the flag since deleting NULL is
safe.
Done.
http://codereview.chromium.org/10807024/diff/2001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (left):
http://codereview.chromium.org/10807024/diff/2001/src/hydrogen-instructions.h#oldcode2292
src/hydrogen-instructions.h:2292: ASSERT(!HEAP->IsAllocationAllowed());
On 2012/07/19 12:47:30, Yang wrote:
It would be great if we could distinguish between the threads to still
check
that the optimizing compile thread does not allocate. Since we never
allocate
anything in the optimizing compile thread, why not have every place
that checks
for allocation_allowed_ to also check that the thread is not the
optimizing
compile thread?
We can't leave in the checks as-is since the JS thread probably has
allowed allocation on the heap by now. But they can be made conditional
on which thread we're executing in.
http://codereview.chromium.org/10807024/diff/2001/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):
http://codereview.chromium.org/10807024/diff/2001/src/ia32/builtins-ia32.cc#newcode80
src/ia32/builtins-ia32.cc:80: __ add(eax, Immediate(Code::kHeaderSize -
kHeapObjectTag));
On 2012/07/19 12:47:30, Yang wrote:
Use the lea instruction and FieldOperand would be easier to read.
Done.
http://codereview.chromium.org/10807024/diff/2001/src/isolate.h
File src/isolate.h (left):
http://codereview.chromium.org/10807024/diff/2001/src/isolate.h#oldcode764
src/isolate.h:764:
On 2012/07/19 12:47:30, Yang wrote:
Stray edit?
Done.
http://codereview.chromium.org/10807024/diff/2001/src/isolate.h
File src/isolate.h (right):
http://codereview.chromium.org/10807024/diff/2001/src/isolate.h#newcode56
src/isolate.h:56: class CompilerProxy;
On 2012/07/19 14:26:31, Yang wrote:
Left over? I can't find any CompilerProxy class.
Yeah, fixed.
http://codereview.chromium.org/10807024/diff/2001/src/isolate.h#newcode1294
src/isolate.h:1294: CompilerProxy* compiler_proxy_;
On 2012/07/19 14:26:31, Yang wrote:
ditto.
Done.
http://codereview.chromium.org/10807024/diff/2001/src/optimizing-compiler-thread.cc
File src/optimizing-compiler-thread.cc (right):
http://codereview.chromium.org/10807024/diff/2001/src/optimizing-compiler-thread.cc#newcode81
src/optimizing-compiler-thread.cc:81: OptimizingCompiler* compiler =
NULL;
On 2012/07/19 12:47:30, Yang wrote:
Maybe OptimizingCompileTask would be a better name than
OptimizingCompiler.
I'm not sure about this -- MakeCrankshaftCode uses OptimizingCompiler
just like a compiler, for instance. It really does have all the state
and the code needed to fully optimize a function.
http://codereview.chromium.org/10807024/diff/2001/src/smart-pointers.h
File src/smart-pointers.h (right):
http://codereview.chromium.org/10807024/diff/2001/src/smart-pointers.h#newcode139
src/smart-pointers.h:139: #endif // V8_SMART_ARRAY_POINTERS_H_
On 2012/07/19 14:26:31, Yang wrote:
You probably mean V8_SMART_POINTERS_H_ in this comment.
Done.
http://codereview.chromium.org/10807024/diff/2001/src/v8.cc
File src/v8.cc (right):
http://codereview.chromium.org/10807024/diff/2001/src/v8.cc#newcode110
src/v8.cc:110: delete isolate;
On 2012/07/19 12:47:30, Yang wrote:
Why move this code?
Added comment.
http://codereview.chromium.org/10807024/diff/2001/src/x64/builtins-x64.cc
File src/x64/builtins-x64.cc (right):
http://codereview.chromium.org/10807024/diff/2001/src/x64/builtins-x64.cc#newcode81
src/x64/builtins-x64.cc:81: __ addq(kScratchRegister,
Immediate(Code::kHeaderSize - kHeapObjectTag));
On 2012/07/19 12:47:30, Yang wrote:
I'd prefer if you use the lea-instruction and FieldOperand here to
compute the
jump target (easier to read).
Done.
http://codereview.chromium.org/10807024/diff/2001/test/mjsunit/array-bounds-check-removal.js
File test/mjsunit/array-bounds-check-removal.js (right):
http://codereview.chromium.org/10807024/diff/2001/test/mjsunit/array-bounds-check-removal.js#newcode28
test/mjsunit/array-bounds-check-removal.js:28: // Flags:
--allow-natives-syntax --expose-gc --noparallel-recompilation
On 2012/07/19 12:47:30, Yang wrote:
Can't we force non-threaded compilation when we use
%OptimizeFunctionOnNextCall?
Or have it stall execution until function has been installed?
I removed this by accident -- %OptimizeFunctionOnNextCall has correct
semantics even when compiling things parallely.
http://codereview.chromium.org/10807024/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
