Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build v8 for Apple Silicon #202

Merged
merged 11 commits into from
Oct 28, 2021
Merged

Build v8 for Apple Silicon #202

merged 11 commits into from
Oct 28, 2021

Conversation

epk
Copy link
Contributor

@epk epk commented Oct 24, 2021

When building v8 on macOS, v8 tooling falls back to Xcode to provide the build toolchain, which supports cross compilation between amd64 and arm64. This PR adds a darwin_arm64 v8 build and changes the CI workflow to automatically build for darwin_arm64

lipo:

lipo -info deps/darwin_arm64/libv8.a
Non-fat file: deps/darwin_arm64/libv8.a is architecture: arm64

Build Logs from my intel Mac building the arm target:

[3128/3133] ../../v8/third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/v8_snapshot/setup-isolate-deserialize.o.d -D_LIBCPP_HAS_NO_ALIGNED_ALLOCATION -DCR_XCODE_VERSION=1310 -DCR_CLANG_REVISION=\"llvmorg-13-init-1559-g01b87444-2\" -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORES=0 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DV8_EMBEDDER_STRING=\"-v8go\" -DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64 -DENABLE_MINOR_MC -DV8_INTL_SUPPORT -DV8_ATOMIC_OBJECT_FIELD_WRITES -DV8_ATOMIC_MARKING_STATE -DV8_ENABLE_LAZY_SOURCE_POSITIONS -DV8_WIN64_UNWINDING_INFO -DV8_ENABLE_REGEXP_INTERPRETER_THREADED_DISPATCH -DV8_SNAPSHOT_COMPRESSION -DV8_ENABLE_WEBASSEMBLY -DV8_COMPRESS_POINTERS -DV8_31BIT_SMIS_ON_64BIT_ARCH -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS -DCPPGC_CAGED_HEAP -DV8_TARGET_ARCH_ARM64 -DV8_HAVE_TARGET_OS -DV8_TARGET_OS_MACOSX -DDISABLE_UNTRUSTED_CODE_MITIGATIONS -DV8_COMPRESS_POINTERS -DV8_31BIT_SMIS_ON_64BIT_ARCH -DV8_DEPRECATION_WARNINGS -DV8_IMMINENT_DEPRECATION_WARNINGS -DCPPGC_CAGED_HEAP -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DUSE_CHROMIUM_ICU=1 -DU_ENABLE_TRACING=1 -DU_ENABLE_RESOURCE_TRACING=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -I../../v8 -Igen -I../../v8 -I../../v8/include -Igen -Igen/include -I../../v8/third_party/icu/source/common -I../../v8/third_party/icu/source/i18n -fno-delete-null-pointer-checks -fno-strict-aliasing -fstack-protector -fcolor-diagnostics -fmerge-all-constants -fcrash-diagnostics-dir=../../v8/tools/clang/crashreports -mllvm -instcombine-lower-dbg-declare=0 -arch arm64 -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -Xclang -fdebug-compilation-dir -Xclang . -no-canonical-prefixes -Wall -Wextra -Wimplicit-fallthrough -Wunreachable-code -Wthread-safety -Wextra-semi -Wunguarded-availability -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-unneeded-internal-declaration -Wno-undefined-var-template -Wno-psabi -Wno-ignored-pragma-optimize -Wno-implicit-int-float-conversion -Wno-final-dtor-non-final-class -Wno-builtin-assume-aligned-alignment -Wno-deprecated-copy -Wno-non-c-typedef-for-linkage -Wno-max-tokens -fno-omit-frame-pointer -g0 -isysroot ../../../../../../../../../Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.0.sdk -mmacosx-version-min=10.11.0 -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -Wmissing-field-initializers -Wunreachable-code -Wshorten-64-to-32 -O3 -fvisibility=default -Wexit-time-destructors -std=c++14 -fno-trigraphs -Wno-trigraphs -stdlib=libc++ -fno-exceptions -fno-rtti -c ../../v8/src/init/setup-isolate-deserialize.cc -o obj/v8_snapshot/setup-isolate-deserialize.o

To build an arm64 binary importing this module from an intel Mac:

$ CGO_ENABLED=1 GOOS=darwin GOARCH=arm64 \
    SDKROOT=$(xcrun --sdk macosx --show-sdk-path) \
    go build -o out/foo ./cmd/foo

See golang/go#44112 for details

closes #171 #54

Comment on lines +11 to +12
// #cgo darwin,amd64 LDFLAGS: -L${SRCDIR}/deps/darwin_x86_64
// #cgo darwin,arm64 LDFLAGS: -L${SRCDIR}/deps/darwin_arm64
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@epk epk changed the title Build v8 for darwin_arm64 Build v8 for Apple Silicon Oct 24, 2021
@epk epk marked this pull request as ready for review October 24, 2021 05:11
Copy link
Owner

@rogchap rogchap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR; great that we can cross-compile for Darwin arm64 🎉

How do we run the unit tests on arm64 🤔

I've just ordered a M1 Mac, so will be able to functional test soon.

.github/workflows/v8build.yml Show resolved Hide resolved
@@ -1,3 +1,3 @@
module rogchap.com/v8go

go 1.12
go 1.16
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely a good idea to update to minimum of 1.16; but just making sure this will be ok for others? @dylanahsmith

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are on 1.17. I think we should support what Go itself supports, so 👍 to this change.

@@ -12,10 +12,10 @@ jobs:
name: Tests on ${{ matrix.go-version }} ${{ matrix.platform }}
strategy:
matrix:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we missing the arch values for this pipeline?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't test against darwin/arm64 without access to a M1 action runner

Run CGO_ENABLED=1 GOOS=darwin GOARCH=arm64 SDKROOT=$(xcrun --sdk macosx --show-sdk-path) go test -v -coverprofile c.out ./...
6
fork/exec /var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/go-build300881077/b001/v8go.test: bad CPU type in executable
7
FAIL	rogchap.com/v8go	0.005s
8
?   	rogchap.com/v8go/deps/darwin_arm64	[no test files]
9
?   	rogchap.com/v8go/deps/darwin_x86_64	[no test files]
10
?   	rogchap.com/v8go/deps/include	[no test files]
11
?   	rogchap.com/v8go/deps/include/cppgc	[no test files]
12
?   	rogchap.com/v8go/deps/include/libplatform	[no test files]
13
?   	rogchap.com/v8go/deps/linux_x86_64	[no test files]
14
?   	rogchap.com/v8go/deps/windows_x86_64	[no test files]
15
FAIL

@epk
Copy link
Contributor Author

epk commented Oct 27, 2021

Tested with a private repo importing this branch on a M1 Mac.

How do we run the unit tests on arm64 🤔

Sadly we won't be able to run tests against darwin/arm64 without access to a M1 runner. However we can test against linux/arm64 with qemu

deps/build.py Outdated Show resolved Hide resolved
deps/build.py Outdated Show resolved Hide resolved
deps/build.py Outdated Show resolved Hide resolved
@epk
Copy link
Contributor Author

epk commented Oct 27, 2021

🎩 with latest changes: https://github.com/epk/v8go/actions/runs/1391408059

@codecov
Copy link

codecov bot commented Oct 28, 2021

Codecov Report

Merging #202 (f4c24fb) into master (3e22137) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #202   +/-   ##
=======================================
  Coverage   95.59%   95.59%           
=======================================
  Files          16       16           
  Lines         545      545           
=======================================
  Hits          521      521           
  Misses         15       15           
  Partials        9        9           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e22137...f4c24fb. Read the comment docs.

Copy link
Collaborator

@dylanahsmith dylanahsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebuild the binary (#216) after merging #138 and pushed that build to this branch along with a changelog entry

go-version: [1.12.17, 1.16.4]
platform: [ubuntu-latest, macos-latest, windows-latest]
go-version: [1.16.8, 1.17.1]
Copy link
Collaborator

@dylanahsmith dylanahsmith Oct 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github is showing "Some checks haven’t completed yet", but then lists the status for Go versions that were updated here to no longer run:

Tests on 1.12.17 ubuntu-latest Expected — Waiting for status to be reported (Required)
Tests on 1.12.17 windows-latest Expected — Waiting for status to be reported (Required)
Tests on 1.16.4 macos-latest Expected — Waiting for status to be reported (Required)
Tests on 1.16.4 ubuntu-latest Expected — Waiting for status to be reported (Required)
Tests on 1.16.4 windows-latest Expected — Waiting for status to be reported (Required)

This is blocking me from merging. @rogchap you could do the merge to workaround this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it looks like the github branch protection rules for the repo need to be updated, since the name of these CI jobs has changed.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Any instructions for m1 builds
3 participants