583199 - Compile WebM on Solaris

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Comment 1

[Ginn Chen]

Attachment: patch

1)
Sun Studio doesn't support anonymous nested union, it is not C standard.
2)
Sun Studio inline asm, __inline keyword, include linux headers, etc.
3)
In Makefile.in, use -xO3 for filter_c.c to avoid a compiler bug.

Only 1) affects other platforms.

yasm works fine on Solaris x86 and x86_64.

Comment 2

[cajbir (:cajbir)]

Comment on attachment 461496 [details] [diff] [review]
patch

Thanks for the patch, Tim is the reviewer/maintainer of media/libvpx. I've passed the review to him.

Comment 3

[Timothy B. Terriberry (:derf)]

Comment on attachment 461496 [details] [diff] [review]
patch

>+# Workaround a bug of Sun Studio (CR 6963410)

I couldn't find this bug report, either through Google or bugs.sun.com . Could you include a direct URL to the issue?

>+diff --git a/media/libvpx/vpx_config.h b/media/libvpx/vpx_config.h
>+--- a/media/libvpx/vpx_config.h
>++++ b/media/libvpx/vpx_config.h

>+diff --git a/media/libvpx/vpx_config_c.c b/media/libvpx/vpx_config_c.c
>+--- a/media/libvpx/vpx_config_c.c
>++++ b/media/libvpx/vpx_config_c.c

vpx_config.h and vpx_config_c.c are our files, and not part of the original libvpx code, so they shouldn't be patched via update.sh (the direct changes below are sufficient). I would, however, suggest adding comments to update.sh (where it says # Config files for x86-linux-gcc, etc.) indicating that these files are now being shared with Solaris.

r+ with the above changes.

>+#elif defined(__SUNPRO_C) || defined(__SUNPRO_CC)
>+#if ARCH_X86_64
>+#define cpuid(func,ax,bx,cx,dx)\
>+    asm volatile (\
>+                  "xchg %rsi, %rbx \n\t" \
>+                  "cpuid           \n\t" \
>+                  "movl %ebx, %edi \n\t" \
>+                  "xchg %rsi, %rbx \n\t" \
>+                  : "=a" (ax), "=D" (bx), "=c" (cx), "=d" (dx) \
>+                  : "a"  (func));
>+#else

Just to satisfy my curiosity: with gcc on x86-64 (but not on x86-32), just adding an =b constraint will case gcc to insert code to save and restore %rbx. Does the Sun Studio compiler not do this also?

You also should submit the changes to libvpx files upstream (http://www.webmproject.org/code/). They may have better solutions for some of these issues (e.g., removing the use of __inline altogether). See either http://www.webmproject.org/code/contribute/submitting-patches/ or http://www.webmproject.org/code/bug-reporting/

Comment 4

[Ginn Chen]

(In reply to comment #3)
> Comment on attachment 461496 [details] [diff] [review]
> patch
> 
> >+# Workaround a bug of Sun Studio (CR 6963410)
> 
> I couldn't find this bug report, either through Google or bugs.sun.com . Could
> you include a direct URL to the issue?

It wil be in bugs.sun.com when it becomes public.

> Just to satisfy my curiosity: with gcc on x86-64 (but not on x86-32), just
> adding an =b constraint will case gcc to insert code to save and restore %rbx.
> Does the Sun Studio compiler not do this also?

Sun Studio 12 didn't.
Sun Studio 12.1 did.
Since we're already here, I'd like to keep it compatible with old compiler.

Comment 5

[Ginn Chen]

Attachment: patch to commit

Comment 6

[Ginn Chen]

http://hg.mozilla.org/mozilla-central/rev/28b218e903b0