Closed
Bug 1364840
Opened 7 years ago
Closed 7 years ago
Build failure for Linux/aarch64 after updating Skia to m59
Categories
(Core :: Graphics, enhancement)
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: m_kato, Assigned: m_kato)
References
Details
Attachments
(2 files)
2 issues - m59 requires SkOpts_crc32.cpp for aarch64 build, so we have to update python script to generate moz.build file - Linux/arm and Linux/aarch64 might use sys/auxv.h to detect CPU feature. So we have to add it to system-headers
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8867637 [details] Bug 1364840 - Part 2. Generate aarch64 build rule to add CRC32 feature on aarch64. https://reviewboard.mozilla.org/r/139224/#review142560
Attachment #8867637 -
Flags: review?(lsalzman) → review+
Updated•7 years ago
|
Blocks: Fennec-ARM64
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8867637 [details] Bug 1364840 - Part 2. Generate aarch64 build rule to add CRC32 feature on aarch64. https://reviewboard.mozilla.org/r/139224/#review143380 ::: gfx/skia/moz.build:742 (Diff revision 1) > SOURCES['skia/src/opts/SkOpts_avx.cpp'].flags += ['-DSK_CPU_SSE_LEVEL=51'] > SOURCES['skia/src/opts/SkOpts_hsw.cpp'].flags += ['-DSK_CPU_SSE_LEVEL=52'] > elif CONFIG['CPU_ARCH'] == 'arm' and CONFIG['GNU_CC']: > CXXFLAGS += CONFIG['NEON_FLAGS'] > +elif CONFIG['CPU_ARCH'] == 'aarch64' and CONFIG['GNU_CC']: > + SOURCES['skia/src/opts/SkOpts_crc32.cpp'].flags += ['-march=armv8-a+crc'] Not all armv8's support the crc extensions.
Attachment #8867637 -
Flags: review-
Comment 5•7 years ago
|
||
mozreview-review |
Comment on attachment 8867636 [details] Bug 1364840 - Part 1. Add sys/auxv.h to system-headers. https://reviewboard.mozilla.org/r/139222/#review143382
Attachment #8867636 -
Flags: review?(mh+mozilla) → review+
Comment 6•7 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #4) > Comment on attachment 8867637 [details] > Bug 1364840 - Part 2. Generate aarch64 build rule to add CRC32 feature on > aarch64. > > https://reviewboard.mozilla.org/r/139224/#review143380 > > ::: gfx/skia/moz.build:742 > (Diff revision 1) > > SOURCES['skia/src/opts/SkOpts_avx.cpp'].flags += ['-DSK_CPU_SSE_LEVEL=51'] > > SOURCES['skia/src/opts/SkOpts_hsw.cpp'].flags += ['-DSK_CPU_SSE_LEVEL=52'] > > elif CONFIG['CPU_ARCH'] == 'arm' and CONFIG['GNU_CC']: > > CXXFLAGS += CONFIG['NEON_FLAGS'] > > +elif CONFIG['CPU_ARCH'] == 'aarch64' and CONFIG['GNU_CC']: > > + SOURCES['skia/src/opts/SkOpts_crc32.cpp'].flags += ['-march=armv8-a+crc'] > > Not all armv8's support the crc extensions. This should only let us compile SkOpts_crc32. Skia uses a runtime check to determine whether to actually use SkOpts_crc32 or not.
Comment 7•7 years ago
|
||
(In reply to Jim Chen [:jchen] [:darchons] from comment #6) > (In reply to Mike Hommey [:glandium] from comment #4) > > Comment on attachment 8867637 [details] > > Bug 1364840 - Part 2. Generate aarch64 build rule to add CRC32 feature on > > aarch64. > > > > https://reviewboard.mozilla.org/r/139224/#review143380 > > > > ::: gfx/skia/moz.build:742 > > (Diff revision 1) > > > SOURCES['skia/src/opts/SkOpts_avx.cpp'].flags += ['-DSK_CPU_SSE_LEVEL=51'] > > > SOURCES['skia/src/opts/SkOpts_hsw.cpp'].flags += ['-DSK_CPU_SSE_LEVEL=52'] > > > elif CONFIG['CPU_ARCH'] == 'arm' and CONFIG['GNU_CC']: > > > CXXFLAGS += CONFIG['NEON_FLAGS'] > > > +elif CONFIG['CPU_ARCH'] == 'aarch64' and CONFIG['GNU_CC']: > > > + SOURCES['skia/src/opts/SkOpts_crc32.cpp'].flags += ['-march=armv8-a+crc'] > > > > Not all armv8's support the crc extensions. > > This should only let us compile SkOpts_crc32. Skia uses a runtime check to > determine whether to actually use SkOpts_crc32 or not. Correct, SkOpts_crc32 is compiled with the feature, but entry into that code is guarded by a runtime check for the CRC32 feature. That's basically how all the SkOpts_foo files work.
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to Lee Salzman [:lsalzman] from comment #7) > Correct, SkOpts_crc32 is compiled with the feature, but entry into that code > is guarded by a runtime check for the CRC32 feature. That's basically how > all the SkOpts_foo files work. Yes, SkCpu.cpp added runtime check to detect new CRC32 feature. So part 1 fix requires on m59.
Updated•7 years ago
|
Attachment #8867637 -
Flags: review-
Pushed by m_kato@ga2.so-net.ne.jp: https://hg.mozilla.org/integration/autoland/rev/de97b072fede Part 1. Add sys/auxv.h to system-headers. r=glandium https://hg.mozilla.org/integration/autoland/rev/bc320d3634ee Part 2. Generate aarch64 build rule to add CRC32 feature on aarch64. r=lsalzman
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/de97b072fede https://hg.mozilla.org/mozilla-central/rev/bc320d3634ee
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•