On 2014/05/08 12:43:42, Yang wrote:
On 2014/05/08 12:42:40, Yang wrote:
> On 2014/05/08 12:40:48, rmcilroy wrote:
> > On 2014/05/08 11:13:33, Yang wrote:
> > > On 2014/05/08 10:47:47, Yang wrote:
> > > > On 2014/05/08 10:47:11, ulan wrote:
> > > > > On 2014/05/08 10:36:53, Yang wrote:
> > > > > > On 2014/05/08 10:17:09, ulan wrote:
> > > > > > > Here is what happens in debug-multiple-breakpoints.js:
> > > > > > >
> > > > > > > 1. A hot function is marked for optimization by replacing
its
code
> > with
> > > > > > > CompileOptimized or CompileOptimizedConcurrent builtin.
> > > > > > > 2. Debug::PrepareForBreakPoints deoptimizes optimized
functions
but
> > > > doesn't
> > > > > > > restore code of functions that were marked for optimization.
> > > > > > > 3. ASSERT_EQ(Code::FUNCTION, function->code()->kind())
triggers
in
> > > > > > > Debug::MaybeRecompileFunctionForDebugging
> > > > > > >
> > > > > > > Yang, is this a bug in Debug::PrepareForBreakPoints that
functions
> > > marked
> > > > > for
> > > > > > > optimization do not get their code restored?
> > > > > >
> > > > > > No. This seems to be a result of the refactoring in
> > > > > >
https://codereview.chromium.org/260423002/diff/40001/src/debug.cc
> > > > > >
> > > > > > The assertion that is triggering has been newly introduced. I
think
we
> > > > should
> > > > > > remove it. What should happen there is that if the function
is on
the
> > > stack
> > > > at
> > > > > > the point of Debug::PrepareForBreakPoints, it should be
recompiled
for
> > > > > debugging
> > > > > > regardless of what it currently is (including being marked by
> builtin).
> > > The
> > > > > only
> > > > > > exception to that rule is that if it already has been
recompiled
for
> > > > > debugging.
> > > > > >
> > > > > > I will make this easier to understand in the future, but for
now,
just
> > > > revert
> > > > > > that part of the refactoring there.
> > > > >
> > > > > Thanks, Yang. So we can just remove the assertions
> > > > > 2006 ASSERT_EQ(Code::FUNCTION, function->code()->kind());
> > > > > 2007 ASSERT_EQ(function->code(), function->shared()->code());
> > > > > in Debug::MaybeRecompileFunctionForDebugging? Because the
function
code
> > will
> > > > be
> > > > > replaced anyway.
> > > >
> > > > Yes.
> > >
> > > Wait. Please check for (Code::FUNCTION == function->code()->kind())
before
> > > checking whether it already has_debug_break_slots(). Otherwise the
latter
> may
> > > assert. Something like
> > > if (Code::FUNCTION == function->code()->kind() &&
> > > function->code()->has_debug_break_slots()) return;
> >
> > Thanks Yang! I guess you mean:
> > if (Code::FUNCTION != function->code()->kind() ||
> > function->code()->has_debug_break_slots()) return;
> > ? I'll give that a go and send out a separate CL if this fixes
things.
>
> No! I don't mean that. In MaybeRecompileFunctionForDebugging, what we
want
to
do
> is to make sure the function code afterwards is unoptimized and contains
debug
> break slots. So only if that's already true, we return prematurely. If
the
> function code kind is not FUNCTION, it could be a builtin, or optimized
code.
In
> either of those cases, we want to recompile.
We could actually rename it to EnsureFunctionHasDebugBreakSlots.
Ulan / Jacob: With the debug fix (which I'll land separately) and the
additional
changed in patchset 5 - which avoids generating too much code in debug mode
by
avoiding reentering AssertStackConsistency from the Push in Abort, and only
generating code AssertStackConsistency if we are using real aborts (since
the
PrintF's in a non-real Abort are massive) - we pass the remaining tests
AFAICT.
I also modified test-spaces to not check if CODE_SPACE == 1 page if
FLAG_debug_code, since we are still at 2 pages after this change due to the
added assert code. PTAL.
https://codereview.chromium.org/271543004/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
