Closed
Bug 1503733
Opened 5 years ago
Closed 5 years ago
Skia m71 merge breaks all tier3 platforms
Categories
(Core :: Graphics, defect, P3)
Tracking
()
RESOLVED
FIXED
mozilla65
Tracking | Status | |
---|---|---|
firefox65 | --- | fixed |
People
(Reporter: awilfox, Assigned: awilfox)
References
Details
(Whiteboard: [gfx-noted])
Attachments
(1 file)
1.60 KB,
patch
|
lsalzman
:
review+
|
Details | Diff | Splinter Review |
Linux/ppc64: 7:22.61 In file included from /var/clean/mozppc/mozilla-unified/gfx/skia/skia/src/core/SkOpts.cpp:39:0: 7:22.61 /var/clean/mozppc/mozilla-unified/gfx/skia/skia/src/opts/SkBitmapFilter_opts.h:895:21: error: missing binary operator before token "(" 7:22.61 #if SK_HAS_ATTRIBUTE(optimize) && defined(SK_RELEASE) 7:22.62 ^ 7:23.96 make[4]: *** [/var/clean/mozppc/mozilla-unified/config/rules.mk:1128: SkOpts.o] Error 1 This is because in SkBitmapFilter_opts.h, there is: #if SK_CPU_SSE_LEVEL >= SK_CPU_SSE_LEVEL_SSE2 .. #elif defined(SK_ARM_HAS_NEON) .. #else .. // There's a bug somewhere here with GCC autovectorization (-ftree-vectorize). We originally // thought this was 32 bit only, but subsequent tests show that some 64 bit gcc compiles // suffer here too. // // Dropping to -O2 disables -ftree-vectorize. GCC 4.6 needs noinline. https://bug.skia.org/2575 #if SK_HAS_ATTRIBUTE(optimize) && defined(SK_RELEASE) #define SK_MAYBE_DISABLE_VECTORIZATION __attribute__((optimize("O2"), noinline)) #else #define SK_MAYBE_DISABLE_VECTORIZATION #endif SK_MAYBE_DISABLE_VECTORIZATION void convolve_horizontally(const unsigned char* srcData, const SkConvolutionFilter1D& filter, unsigned char* outRow, bool hasAlpha) { if (hasAlpha) { ConvolveHorizontally<true>(srcData, filter, outRow); } else { ConvolveHorizontally<false>(srcData, filter, outRow); } } #undef SK_MAYBE_DISABLE_VECTORIZATION ... #endif SK_HAS_ATTRIBUTE was removed from gfx/skia/skia/include/core/SkPreConfig.h when Bug 1502152 (Skia m71) landed. Since this bug only seems to affect GCC 4(?) it may be possible to simple remove that preprocessor block entirely; I also don't believe many people try building Firefox beyond -O2, so that makes it even less risky. I'm going to try a build on Linux/ppc64 with such a change and see how that goes.
Assignee | ||
Comment 1•5 years ago
|
||
Due to Bug 1503749 I haven't been able to ensure this is correct at runtime, but it does allow the build to succeed and I don't see why it wouldn't be correct.
Attachment #9021715 -
Flags: review?(lsalzman)
Updated•5 years ago
|
Attachment #9021715 -
Flags: review?(lsalzman) → review+
Updated•5 years ago
|
Assignee: nobody → AWilcox
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: checkin-needed
OS: Linux → Unspecified
Hardware: Desktop → Unspecified
Whiteboard: [gfx-noted]
Updated•5 years ago
|
Pushed by ebalazs@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/e9b8b0ba18f4 Allow compilation of Skia on non-x86/arm platforms. r=lsalzman
Keywords: checkin-needed
Comment 4•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/e9b8b0ba18f4
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in
before you can comment on or make changes to this bug.
Description
•