New issue
Advanced search Search tips

Issue 616017 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue v8:6105
issue 645094



Sign in to add a comment

GN support for mips and mips64

Project Member Reported by hablich@chromium.org, May 31 2016

Issue description

mips and mips64 are officially supported platforms of V8.
 
is this just for support in v8 standalone specifically? I think GN and //build has most if not all of what is needed already, given that we ship android on mips w/ GN now, right?
I think this is about big endian support, i.e. mips vs. mipsel and mips64 vs mips64el.
Cc: v8-mips-...@googlegroups.com
Blocking: 621726
Blocking: -621726
Labels: -Proj-GN-Migration Proj-GN-Migration-V8
As suggested by laforge@ and a conversation w/ the monorail folks, I'm going to try tracking V8 GN-Migration related issues by *just* using the Proj-GN-Migration-V8 label, and not using blocking/rollup bugs, so that we can use blocking for just tasks that truly need to be completed before other tasks can make progress.
Labels: Pri-2
Lowering priority given that there is no activity for half a year on this bug.
I am curious about the status of this item. If this needs to be done, but there are nobody available, mips support team could try doing it.

I know MIPS simulators compile without problems using GN. How much effort do you think we would need to put into this in order for v8-MIPS to compile on the board?
Cc: dpranke@chromium.org
Any help is welcome! Eventually we need to make this work in order to deprecate gyp.

From the top of my head: The builder https://build.chromium.org/p/client.v8.ports/builders/V8%20Mips%20-%20builder overrides the CC, CXX, etc with the mips toolchain. See e.g. https://build.chromium.org/p/client.v8.ports/builders/V8%20Mips%20-%20builder/builds/8040/steps/gclient%20runhooks/logs/stdio

The toolchain was manually deployed on that bot in some folder.

In gn we can't override CXX and others anymore, but need to create a toolchain definition. Not sure if this should be done chrome-side (https://cs.chromium.org/chromium/src/build/toolchain/) or rather v8-side.

It's also not clear yet how we should bundle the mips toolchain. Maybe upload it to google storage and download it on demand as a hook?
Cc: yangguo@chromium.org
Blocking: v8:6105
So I talked to the people who do MIPS support for Chrome, and they compile v8 with bundled Chrome for some time now without problems. So I guess this shouldn't be too difficult to accomplish.

On GYP, it is possible to compile v8 in the following manner: CLANG + debian sysroot + GOLD linker. In that case we don't need any external tools, so the story about toolchain definition doesn't apply here.

Next step would be to allow v8 compilation with GCC crosscompile toolchain. This is the place where we need to create toolchain definition.

The last step would be adding of big endian support to GN. 
I'm working on compiling v8 using CLANG + debian sysroot + GOLD linker. I managed to compile it and execute d8, but I'm facing some issues and I would like to hear your opinions.

To compile it using this build:
gn gen out.gn/mipsel_debug_clang --args='is_debug=true target_cpu="mipsel" v8_target_cpu="mipsel" mips_arch_variant="r2" is_clang=true target_os="linux" gcc_target_rpath="/home/ci20/sreten/Documents/v8/build/linux/debian_wheezy_mips-sysroot/lib/mipsel-linux-gnu:/home/ci20/sreten/Documents/v8/build/linux/debian_wheezy_mips-sysroot/usr/lib/mipsel-linux-gnu:\$ORIGIN/."'

Currently, I have the following issue:
- Can't add --dynamic-linker argument to ldflags, since (as it seems to me) same flags are used for both preparation of toolchain (in this case clang_x86_v8_mipsel) and for linking of v8 files. 

Is there a way to get this thing done some other way that any of you are familiar with?
I found a way to specify custom dynamic linker by introducing the new variable ldso_path in build/config/gcc/BUILD.gn, the same way it is already done for GYP. We need this in order to compile and execute v8 in our environment (Debian Wheezy sysroot but Debian Jessie on the target). Is it ok to send this patch to code review, or is there some reason we do not want this possibility ?


---------------------------------------
Further technical explanation:

Since I was cross-compiling v8 for ci20 board where is jessie debian sysroot, which is not same as sysroot that is used for v8 (wheezy) I had to use gcc_target_rpath to set additional paths to wheezy library. For the exact same reason I have a need for option --dynamic-linker.

When using jessie sysroot I don't need any of these options to compile it. And gn gen command that should be executed is:
gn gen out.gn/mipsel_debug_jessie --args='is_debug=true target_cpu="mipsel" v8_target_cpu="mipsel" mips_arch_variant="r2" is_clang=true target_os="linux"'

Since jessie is still not supported in v8 I would like to know is it possible to add support for this option in current system as a workaround until it is settled?

In attachment is a diff file that would do the job for us. With this fix, gn gen command that should be executed is:
gn gen out.gn/mipsel_debug_jessie --args='is_debug=true target_cpu="mipsel" v8_target_cpu="mipsel" mips_arch_variant="r2" is_clang=true target_os="linux" gcc_target_rpath="/home/ci20/sreten/Documents/v8/build/linux/debian_wheezy_mips-sysroot/lib/mipsel-linux-gnu:/home/ci20/sreten/Documents/v8/build/linux/debian_wheezy_mips-sysroot/usr/lib/mipsel-linux-gnu:\$ORIGIN/." ldso_path="/home/ci20/sreten/Documents/v8/build/linux/lib/mipsel-linux-gnu/ld-2.13.so"'
add_ldso.diff
695 bytes Download
btw, note that this bug is mainly about mips not, mipsel

of course, making your toolchain for mipsel work is great as well!
After getting familiar with GN, I started working on mips and mips64 support. I would love to hear your opinion about my work and whether this should be placed on code review.

I have made changes in Chromium part (diff file named big_endian_support). Also, there should be made changes on v8 side as well (diff file named v8_big_endian_support).

I have tested this on both qemu and boards and it works fine.

big_endian_support.diff
2.8 KB Download
v8_big_endian_support.diff
2.0 KB Download
It's easier to review the changes as CLs on codereview.chromium.org than it is when attaching diffs to a bug, so it'd be great if you could upload them.

You should also look at the following CLs:

https://codereview.chromium.org/2809963004/
https://codereview.chromium.org/2815453004/
https://codereview.chromium.org/2812173002/

which is adding AIX, power, and s390 support, and also attacks the byteorder problem :).
Thanks for the reply.

I have uploaded CL:

https://codereview.chromium.org/2875553003/

As I can see, CL that is part of v8 has not passed trybots and hasn't landed yet, so I believe I should wait for it to land before I make any changes on my part.

Comment 19 Deleted

whether add support for building chromium on mips64el using GN???
0001-BUILD-gn-add-mips64el-support.patch
1.7 KB Download
@wqinglll - can you post that patch as a CL so that we can follow the normal contributing processes?

Comment 22 by wqing...@gmail.com, Jul 10 2017

Do I need to new a issue again or post that patch as a CL follow this bug= 616017  directlty???
The patch you are posting is not related to this issue. Your patch enabled native compilation on MIPS64 little endian. This issue is related to big endian compilation

Comment 24 by wqing...@gmail.com, Jul 10 2017

I new  issue 740468 , please to look at,thanks!
Blocking: 645094
Cc: -jochen@chromium.org
Project Member

Comment 27 by bugdroid1@chromium.org, Jul 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/97f3e874313bd9efc69b187b37879de500837ab8

commit 97f3e874313bd9efc69b187b37879de500837ab8
Author: Michael Achenbach <machenbach@chromium.org>
Date: Tue Jul 11 06:45:57 2017

V8: Switch mips builder to GN

This switches the mips (big endian) builder to GN on the infra side.

The bot has no MB configs due to several hacks, which are now removed
too.

In a follow up, we'll migrate this to MB on the V8 side as well.

Bug:  616017 
Change-Id: I39bcce1883a1d2ff52025142c5f6ebca9098785a
Reviewed-on: https://chromium-review.googlesource.com/565292
Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/97f3e874313bd9efc69b187b37879de500837ab8/scripts/slave/recipe_modules/v8/api.py
[modify] https://crrev.com/97f3e874313bd9efc69b187b37879de500837ab8/scripts/slave/README.recipes.md
[modify] https://crrev.com/97f3e874313bd9efc69b187b37879de500837ab8/scripts/slave/recipe_modules/v8/builders.py
[modify] https://crrev.com/97f3e874313bd9efc69b187b37879de500837ab8/scripts/slave/recipes/v8.py
[modify] https://crrev.com/97f3e874313bd9efc69b187b37879de500837ab8/scripts/slave/recipe_modules/v8/chromium_config.py
[modify] https://crrev.com/97f3e874313bd9efc69b187b37879de500837ab8/scripts/slave/recipe_modules/chromium/api.py
[modify] https://crrev.com/97f3e874313bd9efc69b187b37879de500837ab8/scripts/slave/recipes/v8.expected/full_client_v8_ports_V8_Mips___builder.json
[modify] https://crrev.com/97f3e874313bd9efc69b187b37879de500837ab8/scripts/slave/recipe_modules/v8/config.py

Project Member

Comment 28 by bugdroid1@chromium.org, Jul 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/f1c4d95f92424700f99313f228580e4e205c89d7

commit f1c4d95f92424700f99313f228580e4e205c89d7
Author: Michael Achenbach <machenbach@chromium.org>
Date: Tue Jul 11 07:05:49 2017

Revert "V8: Switch mips builder to GN"

This reverts commit 97f3e874313bd9efc69b187b37879de500837ab8.

Reason for revert: Compilation failures:
https://build.chromium.org/p/client.v8.ports/builders/V8%20Mips%20-%20builder/builds/10651

Original change's description:
> V8: Switch mips builder to GN
> 
> This switches the mips (big endian) builder to GN on the infra side.
> 
> The bot has no MB configs due to several hacks, which are now removed
> too.
> 
> In a follow up, we'll migrate this to MB on the V8 side as well.
> 
> Bug:  616017 
> Change-Id: I39bcce1883a1d2ff52025142c5f6ebca9098785a
> Reviewed-on: https://chromium-review.googlesource.com/565292
> Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Commit-Queue: Michael Achenbach <machenbach@chromium.org>

TBR=dpranke@chromium.org,machenbach@chromium.org,yangguo@chromium.org,tandrii@chromium.org,sreten.kovacevic@imgtec.com

Change-Id: I924eebdd420304273c002f97e5b7e99f35c1c20d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  616017 
Reviewed-on: https://chromium-review.googlesource.com/566598
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/f1c4d95f92424700f99313f228580e4e205c89d7/scripts/slave/recipe_modules/v8/api.py
[modify] https://crrev.com/f1c4d95f92424700f99313f228580e4e205c89d7/scripts/slave/README.recipes.md
[modify] https://crrev.com/f1c4d95f92424700f99313f228580e4e205c89d7/scripts/slave/recipe_modules/v8/builders.py
[modify] https://crrev.com/f1c4d95f92424700f99313f228580e4e205c89d7/scripts/slave/recipes/v8.py
[modify] https://crrev.com/f1c4d95f92424700f99313f228580e4e205c89d7/scripts/slave/recipe_modules/v8/chromium_config.py
[modify] https://crrev.com/f1c4d95f92424700f99313f228580e4e205c89d7/scripts/slave/recipe_modules/chromium/api.py
[modify] https://crrev.com/f1c4d95f92424700f99313f228580e4e205c89d7/scripts/slave/recipes/v8.expected/full_client_v8_ports_V8_Mips___builder.json
[modify] https://crrev.com/f1c4d95f92424700f99313f228580e4e205c89d7/scripts/slave/recipe_modules/v8/config.py

Owner: machenb...@chromium.org
Status: Assigned (was: Available)
Next attempt to migrate the mips BE builder after  https://crbug.com/632390  was resolved.
Project Member

Comment 30 by bugdroid1@chromium.org, Feb 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/49bce85225c360e2ae0d96f66cbdd19cf3e55bfa

commit 49bce85225c360e2ae0d96f66cbdd19cf3e55bfa
Author: Michael Achenbach <machenbach@chromium.org>
Date: Thu Feb 08 09:51:05 2018

[build] Add MB config for mips builder

TBR=yangguo@chromium.org
NOTRY=true

Bug:  chromium:616017 
Change-Id: I48a29bc96e2c7ae54f4b5bbb6790db1d8bfccab4
Reviewed-on: https://chromium-review.googlesource.com/908288
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#51169}
[modify] https://crrev.com/49bce85225c360e2ae0d96f66cbdd19cf3e55bfa/infra/mb/mb_config.pyl

Project Member

Comment 31 by bugdroid1@chromium.org, Feb 8 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/4db374bef87092e603ec1f579cddbc66f1d569ad

commit 4db374bef87092e603ec1f579cddbc66f1d569ad
Author: Michael Achenbach <machenbach@chromium.org>
Date: Thu Feb 08 10:48:05 2018

V8: Switch mips builder to MB

This is a partial reland of:
https://crrev.com/c/565292

This switches the mips builder to MB and hence to GN after:
https://crrev.com/c/908288

This makes use of the bundled mips toolchain on the V8 side and compiles
without goma.

Bug:  616017 
Change-Id: I62ee97a6b2ee19ad55351117642ae74d70cbb5c1
Reviewed-on: https://chromium-review.googlesource.com/908329
Reviewed-by: Sergiy Byelozyorov <sergiyb@chromium.org>
Commit-Queue: Michael Achenbach <machenbach@chromium.org>

[modify] https://crrev.com/4db374bef87092e603ec1f579cddbc66f1d569ad/scripts/slave/recipe_modules/v8/api.py
[modify] https://crrev.com/4db374bef87092e603ec1f579cddbc66f1d569ad/scripts/slave/README.recipes.md
[modify] https://crrev.com/4db374bef87092e603ec1f579cddbc66f1d569ad/scripts/slave/recipe_modules/v8/builders.py
[modify] https://crrev.com/4db374bef87092e603ec1f579cddbc66f1d569ad/scripts/slave/recipes/v8.py
[modify] https://crrev.com/4db374bef87092e603ec1f579cddbc66f1d569ad/scripts/slave/recipe_modules/v8/config.py
[modify] https://crrev.com/4db374bef87092e603ec1f579cddbc66f1d569ad/scripts/slave/recipe_modules/chromium/api.py
[modify] https://crrev.com/4db374bef87092e603ec1f579cddbc66f1d569ad/scripts/slave/recipes/v8.expected/full_client_v8_ports_V8_Mips___builder.json

Status: Fixed (was: Assigned)
The mips BE builder migrated to GN and tests seem to pass on the BE boards.

Sign in to add a comment