Closed
Bug 1503733
Opened 7 years ago
Closed 7 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•7 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•7 years ago
|
Attachment #9021715 -
Flags: review?(lsalzman) → review+
Updated•7 years ago
|
Assignee: nobody → AWilcox
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Keywords: checkin-needed
OS: Linux → Unspecified
Hardware: Desktop → Unspecified
Whiteboard: [gfx-noted]
Updated•7 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•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla65
You need to log in
before you can comment on or make changes to this bug.
Description
•