1503733 - Skia m71 merge breaks all tier3 platforms

Status: RESOLVED FIXED

(Core :: Graphics, defect, P3)

Description

[A. Wilcox [:awilfox]]

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.

Comment 1

[A. Wilcox [:awilfox]]

Attachment: Allow compilation of Skia on non-x86/arm platforms

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.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e9b8b0ba18f4
Allow compilation of Skia on non-x86/arm platforms. r=lsalzman

Comment 4

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/e9b8b0ba18f4