Re: First draft of the sh4 port (issue 11275184)

89 views
Skip to first unread message

remi.du...@st.com

unread,
Nov 8, 2012, 10:52:54 AM11/8/12
to jkum...@chromium.org, v8-...@googlegroups.com
Hello,

do you have any comment on this (really) big patch. Any comments, questions
dans
remarks are obviously welcome.


Regards

--
Rémi Duraffort

https://codereview.chromium.org/11275184/

remi.du...@st.com

unread,
Nov 12, 2012, 8:42:55 AM11/12/12
to jkum...@chromium.org, v8-...@googlegroups.com
Hello everyone,

I'm wondering who is the right reviewer for this patch and what to do in
order
to have this patch fully reviewed (I know this a big patch and that it take
a
lot of time to review) ?

jkum...@chromium.org

unread,
Nov 12, 2012, 10:35:47 AM11/12/12
to remi.du...@st.com, v8-...@googlegroups.com
The platform-independent changes in patch set 2 look good.

I don't think anyone is planning to review the SH4-specific changes in
detail,
as AFAIK nobody on the team knows the SH4 architecture well enough to spot
any
mistakes.

The team has discussed this topic, and before we land SH4 support, it would
be
nice to understand the motivation behind the port in order for us to be
able to
decide to what extent we should support it.

Basically, there are two levels of support: there are the ports that
Google's V8
team currently fully supports and maintains (ia32, x64, ARM), and
there's "basic
support" (MIPS) where the code lives in our repository and we work with its
maintainers, e.g. by landing their patches and communicating with them about
issues or upcoming plans such as branch points or significant architectural
changes. We currently don't have any plans to extend the set of platforms
in the
former group. For the latter to work, the platform port must be in a shape
where
it fits into our infrastructure and workflow: we must at least be able to
compile it and run the test suite, which means it must have a simulator and
the
test expectations must be up to date so that the test suite passes. We're
not
willing to introduce dependencies on third-party tools for this, e.g. QEmu.

Before you reach the "basic support" stage, an option that could work quite
well
for you is to have a github clone of V8 that tracks our mirror. That way
you'll
be on top of the latest sources and can iterate on your port with minimal
latency.

Once you have all of the code in place (simulator, anything missing to make
the
majority of the tests work, expectation file for the others), we will
review the
style and general structure of the SH4 specific code. This may take a few
weeks.
To be clear, once the patch is approved, we will land it into the V8 tree,
but
it will continue to be your responsibility to make sure it keeps working and
stays up to date, and we reserve the right to remove the port at a future
date
if you are no longer able to maintain it.

https://codereview.chromium.org/11275184/

remi.du...@st.com

unread,
Nov 14, 2012, 9:23:51 AM11/14/12
to jkum...@chromium.org, v8-...@googlegroups.com
> I don't think anyone is planning to review the SH4-specific changes in
> detail,
> as AFAIK nobody on the team knows the SH4 architecture well enough to
> spot any
> mistakes.

The sounds logical.


> The team has discussed this topic, and before we land SH4 support, it
> would be
> nice to understand the motivation behind the port in order for us to be
> able
to
> decide to what extent we should support it.

We ported v8 on sh4 because that's something that some of our clients want.
This
is mainly for a faster WebKit or just to run node.js.

I don't think the first support level is the right one for sh4 as I will
stay as
maintainer of the port. So "basic support" look appropriate.



> [...]. For the latter to work, the platform port must be in a shape where
> it fits into our infrastructure and workflow: we must at least be able to
> compile it and run the test suite, which means it must have a simulator
> and
the
> test expectations must be up to date so that the test suite passes. We're
> not
> willing to introduce dependencies on third-party tools for this, e.g.
> QEmu.

I guess that providing a tar file with everythind that you need in order to
build and test v8 for sh4 is not enought to change this?


> Once you have all of the code in place (simulator, anything missing to
> make
the
> majority of the tests work, expectation file for the others), we will
> review
the
> style and general structure of the SH4 specific code. This may take a few
weeks.
> To be clear, once the patch is approved, we will land it into the V8
> tree, but
> it will continue to be your responsibility to make sure it keeps working
> and
> stays up to date, and we reserve the right to remove the port at a future
> date
> if you are no longer able to maintain it.

Sound ok too.


I will come back to you when I have the simulator for sh4.

Thanks for the review.
Reply all
Reply to author
Forward
0 new messages