Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2172653002:
MIPS: Fix mksnapshot on big-endian. (Closed)
|
Created:
4 years, 5 months ago by akos.palfi.imgtec Modified:
4 years, 4 months ago CC:
v8-reviews_googlegroups.com, rmcilroy Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionMIPS: Fix mksnapshot on big-endian.
Paritally revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760 and introduce a new separate variable for the mkpeephole.
On big-endian MIPS, qemu is used to build the snapshot,
because there's no simulator support for big-endian MIPS.
BUG=
Committed: https://crrev.com/928d2395c3fdf836cf9961cde96e6b274a6b1e20
Committed: https://crrev.com/45d6909e7b5cfb978abf63fba9042df9e6cbfd00
Cr-Original-Commit-Position: refs/heads/master@{#38103}
Cr-Commit-Position: refs/heads/master@{#38165}
Patch Set 1 #Patch Set 2 : Build mksnapshot for target on mips big-endian. #Patch Set 3 : Fix presubmit. #Patch Set 4 : Fix chromium gyp build. #
Messages
Total messages: 41 (24 generated)
akos.palfi@imgtec.com changed reviewers: + oth@chromium.org
PTAL.
Description was changed from ========== MIPS: revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= ========== to ========== MIPS: Revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= ==========
The CQ bit was checked by oth@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/07/21 16:53:10, akos.palfi.imgtec wrote: > PTAL. Does a clean build work for you locally with this change? If it breaks the bot and that's okay for the MIPS folk, then I'm fine with this CL. The line that this CL proposes removing enabled building and running the mkpeephole program during the build on the big endian MIPS builder in the v8 waterfall: https://build.chromium.org/p/client.v8.ports/builders/V8%20Mips%20-%20builder... > It seems the change in standalone.gypi breaks the mips (big-endian) build when > snapshot is enabled. On big-endian mips we build the mksnapshot for the target > and execute it on qemu (since we haven't implemented big-endian simulator) to > generate the snapshot. With your change now it is built for host which is > little-endian and thus the generated snapshot can't be loaded. Alternatives here might be: 1) build mksnapshot for the target architecture by changing the snapshot build target to be keyed off a variable other than want_separate_host_toolset. 2) build with snapshot off until the MIPS build process can run mksnapshot on the build host.
On 2016/07/22 08:30:47, oth wrote: > On 2016/07/21 16:53:10, akos.palfi.imgtec wrote: > > PTAL. > > Does a clean build work for you locally with this change? If it breaks the bot > and that's okay for the MIPS folk, then I'm fine with this CL. > > The line that this CL proposes removing enabled building and running the > mkpeephole > program during the build on the big endian MIPS builder in the v8 waterfall: > > https://build.chromium.org/p/client.v8.ports/builders/V8%20Mips%20-%20builder... > > > It seems the change in standalone.gypi breaks the mips (big-endian) build when > > snapshot is enabled. On big-endian mips we build the mksnapshot for the target > > and execute it on qemu (since we haven't implemented big-endian simulator) to > > generate the snapshot. With your change now it is built for host which is > > little-endian and thus the generated snapshot can't be loaded. > > Alternatives here might be: > > 1) build mksnapshot for the target architecture by changing the snapshot build > target to > be keyed off a variable other than want_separate_host_toolset. > > 2) build with snapshot off until the MIPS build process can run mksnapshot on > the build host. I don't think we can break our MIPS bot. Either of these solutions seem reasonable though. Any idea how PPC do their similar big-endian build?
On 2016/07/22 09:07:37, rmcilroy wrote: > On 2016/07/22 08:30:47, oth wrote: > > On 2016/07/21 16:53:10, akos.palfi.imgtec wrote: > > > PTAL. > > > > Does a clean build work for you locally with this change? If it breaks the bot > > and that's okay for the MIPS folk, then I'm fine with this CL. > > > > The line that this CL proposes removing enabled building and running the > > mkpeephole > > program during the build on the big endian MIPS builder in the v8 waterfall: > > > > > https://build.chromium.org/p/client.v8.ports/builders/V8%20Mips%20-%20builder... > > > > > It seems the change in standalone.gypi breaks the mips (big-endian) build > when > > > snapshot is enabled. On big-endian mips we build the mksnapshot for the > target > > > and execute it on qemu (since we haven't implemented big-endian simulator) > to > > > generate the snapshot. With your change now it is built for host which is > > > little-endian and thus the generated snapshot can't be loaded. > > > > Alternatives here might be: > > > > 1) build mksnapshot for the target architecture by changing the snapshot build > > target to > > be keyed off a variable other than want_separate_host_toolset. > > > > 2) build with snapshot off until the MIPS build process can run mksnapshot on > > the build host. > > I don't think we can break our MIPS bot. Either of these solutions seem > reasonable though. Any idea how PPC do their similar big-endian build? Actually, I have no idea how PPC guys generate the big-endian snapshot, but maybe they build it natively. AFAIK, the buildbot (V8 Mips - builder) already compiles the big-endian mips build with snapshot=off option (because it doesn't have qemu to run the mksnapshot), so the easiest solution would be to build the mkpeephole for host, but keep the mksnapshot to be compiled for target. For big-endian mips the mksnapshot must be compiled for target, because building it for host will always generate an incorrect snapshot, since the simulator support for big-endian mips is missing. I've uploaded a new patch which adds a separate variable for the mkpeephole, PTAL.
The CQ bit was checked by akos.palfi@imgtec.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
oth@chromium.org changed reviewers: + machenbach@google.com
+machenbach for build approval.
machenbach@chromium.org changed reviewers: + machenbach@chromium.org
lgtm
Maybe a less cryptic commit message title? The current title could be part of the details.
Description was changed from ========== MIPS: Revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= ========== to ========== MIPS: Fix mksnapshot on big-endian. Paritally revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760 and introduce a new separate variable for the mkpeephole. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= ==========
On 2016/07/27 13:59:27, Michael Achenbach (slow) wrote: > Maybe a less cryptic commit message title? The current title could be part of > the details. Thanks, done!
The CQ bit was checked by akos.palfi@imgtec.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== MIPS: Fix mksnapshot on big-endian. Paritally revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760 and introduce a new separate variable for the mkpeephole. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= ========== to ========== MIPS: Fix mksnapshot on big-endian. Paritally revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760 and introduce a new separate variable for the mkpeephole. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== MIPS: Fix mksnapshot on big-endian. Paritally revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760 and introduce a new separate variable for the mkpeephole. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= ========== to ========== MIPS: Fix mksnapshot on big-endian. Paritally revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760 and introduce a new separate variable for the mkpeephole. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= Committed: https://crrev.com/928d2395c3fdf836cf9961cde96e6b274a6b1e20 Cr-Commit-Position: refs/heads/master@{#38103} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/928d2395c3fdf836cf9961cde96e6b274a6b1e20 Cr-Commit-Position: refs/heads/master@{#38103}
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/2182633010/ by machenbach@chromium.org. The reason for reverting is: Blocks the roll: https://codereview.chromium.org/2189443003/ Doesn't work with the last chromium gyp bot..
Message was sent while issue was closed.
Fixed the chromium gyp build, PTAL again.
Message was sent while issue was closed.
Description was changed from ========== MIPS: Fix mksnapshot on big-endian. Paritally revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760 and introduce a new separate variable for the mkpeephole. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= Committed: https://crrev.com/928d2395c3fdf836cf9961cde96e6b274a6b1e20 Cr-Commit-Position: refs/heads/master@{#38103} ========== to ========== MIPS: Fix mksnapshot on big-endian. Paritally revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760 and introduce a new separate variable for the mkpeephole. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= Committed: https://crrev.com/928d2395c3fdf836cf9961cde96e6b274a6b1e20 Cr-Commit-Position: refs/heads/master@{#38103} ==========
The CQ bit was checked by akos.palfi@imgtec.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by akos.palfi@imgtec.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== MIPS: Fix mksnapshot on big-endian. Paritally revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760 and introduce a new separate variable for the mkpeephole. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= Committed: https://crrev.com/928d2395c3fdf836cf9961cde96e6b274a6b1e20 Cr-Commit-Position: refs/heads/master@{#38103} ========== to ========== MIPS: Fix mksnapshot on big-endian. Paritally revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760 and introduce a new separate variable for the mkpeephole. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= Committed: https://crrev.com/928d2395c3fdf836cf9961cde96e6b274a6b1e20 Cr-Commit-Position: refs/heads/master@{#38103} ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== MIPS: Fix mksnapshot on big-endian. Paritally revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760 and introduce a new separate variable for the mkpeephole. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= Committed: https://crrev.com/928d2395c3fdf836cf9961cde96e6b274a6b1e20 Cr-Commit-Position: refs/heads/master@{#38103} ========== to ========== MIPS: Fix mksnapshot on big-endian. Paritally revert standalone.gypi changes in a451bd1a687987a7056d7847ba14aefd195dd760 and introduce a new separate variable for the mkpeephole. On big-endian MIPS, qemu is used to build the snapshot, because there's no simulator support for big-endian MIPS. BUG= Committed: https://crrev.com/928d2395c3fdf836cf9961cde96e6b274a6b1e20 Committed: https://crrev.com/45d6909e7b5cfb978abf63fba9042df9e6cbfd00 Cr-Original-Commit-Position: refs/heads/master@{#38103} Cr-Commit-Position: refs/heads/master@{#38165} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/45d6909e7b5cfb978abf63fba9042df9e6cbfd00 Cr-Commit-Position: refs/heads/master@{#38165} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
