On 2014/05/20 13:09:46, Weiliang wrote:
On 2014/05/20 12:23:28, danno wrote:
>
https://codereview.chromium.org/293743005/diff/1/src/code-stubs-hydrogen.cc
> File src/code-stubs-hydrogen.cc (right):
>
>
https://codereview.chromium.org/293743005/diff/1/src/code-stubs-hydrogen.cc#newcode336
> src/code-stubs-hydrogen.cc:336: #if !defined(V8_TARGET_ARCH_X87)
> On 2014/05/20 08:41:06, Weiliang wrote:
> > Yes, Thanks a lot for both of you comments.
> >
> > To best solve this issue, we would better fix the X87 register
allocator
to
> > avoid spilling the double registers. But currently X87 only has
rudimentary
> > X87Stack tracking and in general it cannot handle phi-nodes (comments
from
> > code).I did a very initial try to enhance it, but it seems to take
some
time.
> > After considering that this issue only break "x87.debug snapshot=on",
I
can
> > remove the code here and hope the CL could land firstly. And then we
will
> > enhance the register allocator to fix this issue. Is it OK?
> > On 2014/05/20 08:18:55, danno wrote:
> > > On 2014/05/20 05:44:02, Weiliang wrote:
> > > > x87 is more easy to spill the double register, so disable it.
> > >
> > > Yes, this platform-specific define here is a no-go. This is a good
example
> why
> > > we removed the x87 port in the first place, precisely because it is
more
> work
> > to
> > > ensure that there is no frame generated on performance-sensitive
stubs,
and
> > > since x87 ins't a core platform, it wasn't worth investing the
effort.
In
> any
> > > case, you'll need to solve this in a platform independent way, e.g.
fixing
> the
> > > x87 register allocator to avoid having to generate the frame.
> >
>
> I think it's fine to only support snapshot=off for now for x87, that
was the
> case for arm64 for a while before the port was complete. Yes, and please
remove
> the #ifdef here.
Done. Thanks a lot.
Actually snapshot=on is ok for release build, just fail for debug build
because
of the blew ASSERT on lithium.cc
ASSERT(!(Serializer::enabled(info()->isolate()) &&
info()->GetMustNotHaveEagerFrame() &&
generator.NeedsEagerFrame()));
Upload patch set 3
https://codereview.chromium.org/293743005/
--
--
v8-dev mailing list
[email protected]
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 [email protected].
For more options, visit https://groups.google.com/d/optout.
