Enable partial DEPS recursion into V8 source tree
Reported by
martyn.c...@arm.com,
May 4 2017
|
||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Steps to reproduce the problem: 1. 2. 3. What is the expected behavior? What went wrong? ARM64 NEON support in V8 depends on an external git repository for its simulator tests, as the files are large and infrequently changed. When Chromium pulls in V8 via DEPS, it doesn't pull in V8's own DEPS, and thus fails to build due to the header files from the test repository being missing. This causes the autoroll to fail for ARM64: https://codereview.chromium.org/2820753003/ https://codereview.chromium.org/2818313002/ I plan to solve the problem in a similar way to the ANGLE project within Chromium, viz: 1. Submit a DEPS.chromium file to V8 containing the traces repository. At this point, it would do nothing. 2. Submit a patch to Chromium's DEPS file, that uses recursion on DEPS.chromium, in the same way as ANGLE. This would start pulling traces into a directory within Chromium's V8, but should have no effect otherwise. 3. Submit the NEON patch to V8 with the main DEPS change. This makes NEON work in V8. 4. Watch, as V8 is rolled into Chromium and the recursive DEPS.chromium file causes the traces to be pulled into Chromium too. Did this work before? N/A Chrome version: <Copy from: 'about:version'> Channel: n/a OS Version: Flash Version:
,
May 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/f8a6c6c48e79ef4267d03c80aa768e8362341e4a commit f8a6c6c48e79ef4267d03c80aa768e8362341e4a Author: martyn.capewell <martyn.capewell@arm.com> Date: Mon May 15 15:23:48 2017 Add DEPS.chromium for recursive DEPS tracking. DEPS.chromium allows the Chromium build system's DEPS to recurse into V8's own dependencies. Initially, this is populated with some tests files for the ARM64 simulator. BUG=chromium:718439 Review-Url: https://codereview.chromium.org/2880293002 Cr-Commit-Position: refs/heads/master@{#45310} [add] https://crrev.com/f8a6c6c48e79ef4267d03c80aa768e8362341e4a/DEPS.chromium
,
May 18 2017
Chromium team have rejected including the trace files in their dependencies: https://codereview.chromium.org/2889663002/ So instead, I'm trying to conditionally include the simulator tests in V8 depending on whether the build is inside Chromium.
,
May 19 2017
+jochen, who supports making building v8 stand-alone and inside of chromium equal. Conditionally including things, dependent on stand-alone vs. chromium would violate this. But as it seems we can't make everybody happy here.
,
May 19 2017
I agree that we'll not want v8 developers to download >70mb of additional stuff. Also, third-party sources need to go into a third_party directory, and opensource approval, and a README file and a LICENSE file etc etc.. can we please revert the v8 side changes while this is all being figured out?
,
May 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/822442f0f64f4f5caf7936fd401f8f7fa8f2e771 commit 822442f0f64f4f5caf7936fd401f8f7fa8f2e771 Author: martyn.capewell <martyn.capewell@arm.com> Date: Fri May 19 10:39:39 2017 Revert of Add DEPS.chromium for recursive DEPS tracking. (patchset #1 id:1 of https://codereview.chromium.org/2880293002/ ) Reason for revert: Going a different way with this, as Chromium don't want the additional files. Original issue's description: > Add DEPS.chromium for recursive DEPS tracking. > > DEPS.chromium allows the Chromium build system's DEPS to recurse into V8's own > dependencies. Initially, this is populated with some tests files for the ARM64 > simulator. > > BUG=chromium:718439 > > Review-Url: https://codereview.chromium.org/2880293002 > Cr-Commit-Position: refs/heads/master@{#45310} > Committed: https://chromium.googlesource.com/v8/v8/+/f8a6c6c48e79ef4267d03c80aa768e8362341e4a TBR=machenbach@chromium.org,bmeurer@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG=chromium:718439 Review-Url: https://codereview.chromium.org/2891323002 Cr-Commit-Position: refs/heads/master@{#45420} [delete] https://crrev.com/ad7caee4279af9244792f86a958b8c81fdb189b4/DEPS.chromium
,
May 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3 commit fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3 Author: martyn.capewell <martyn.capewell@arm.com> Date: Wed May 31 13:58:43 2017 Reland of Reland of "ARM64: Add NEON support" This reverts commit c5aad5f284b2a28b33a899cb0c5716cfbe2f6405 The CL was reverted due to missing Chromium dependencies. This commit removes the simulator trace-based tests, and the associated header file dependencies, previously pulled in by DEPS. The NEON support now has only hand-written tests, in test-assembler-arm64.cc. The remaining tests can be added in a later patch. BUG=chromium:718439 Original issue's description: > Reland "ARM64: Add NEON support" > > This reverts commit cc047635ff54ce19b12cc91978971795f670767d. > The CL was reverted due to a missing DEPS mirror. > > Original issue's description: > > ARM64: Add NEON support > > > > Add assembler, disassembler and simulator support for NEON in the ARM64 backend. > > > > BUG= > > > > Review-Url: https://codereview.chromium.org/2622643005 > > Cr-Commit-Position: refs/heads/master@{#44306} > > BUG= > > Review-Url: https://codereview.chromium.org/2812573003 > Cr-Commit-Position: refs/heads/master@{#44652} Review-Url: https://codereview.chromium.org/2896303003 Cr-Commit-Position: refs/heads/master@{#45633} [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/BUILD.gn [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/assembler-arm64-inl.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/assembler-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/assembler-arm64.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/code-stubs-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/constants-arm64.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/decoder-arm64-inl.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/decoder-arm64.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/deoptimizer-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/disasm-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/disasm-arm64.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/instructions-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/instructions-arm64.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/instrument-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/macro-assembler-arm64-inl.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/macro-assembler-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/macro-assembler-arm64.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/simulator-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/simulator-arm64.h [add] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/simulator-logic-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/utils-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/arm64/utils-arm64.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/compiler/arm64/code-generator-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/compiler/arm64/instruction-selector-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/crankshaft/arm64/delayed-masm-arm64-inl.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/crankshaft/arm64/delayed-masm-arm64.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/crankshaft/arm64/lithium-codegen-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/crankshaft/arm64/lithium-codegen-arm64.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/crankshaft/arm64/lithium-gap-resolver-arm64.h [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/src/v8.gyp [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/test/cctest/test-assembler-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/test/cctest/test-disasm-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/test/cctest/test-utils-arm64.cc [modify] https://crrev.com/fc3f29d329c60c24cca6cddb074dd05f0ee5f0a3/test/cctest/test-utils-arm64.h |
||
►
Sign in to add a comment |
||
Comment 1 by machenb...@chromium.org
, May 4 2017Components: Build
Labels: -Via-Wizard-Other
Owner: machenb...@chromium.org
Status: Assigned (was: Unconfirmed)