Compile WebM on Solaris

RESOLVED FIXED in mozilla2.0b4

Status

()

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: ginnchen+exoracle, Assigned: ginnchen+exoracle)

Tracking

Trunk
mozilla2.0b4
All
OpenSolaris
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Comment hidden (empty)
(Assignee)

Comment 1

8 years ago
Created attachment 461496 [details] [diff] [review]
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.
Attachment #461496 - Flags: review?(chris.double)

Comment 2

8 years ago
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.
Attachment #461496 - Flags: review?(chris.double) → review?(tterribe)
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/
Attachment #461496 - Flags: review?(tterribe) → review+
(Assignee)

Comment 4

8 years ago
(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.
(Assignee)

Comment 5

8 years ago
Created attachment 462749 [details] [diff] [review]
patch to commit
Attachment #462749 - Flags: approval2.0?
Attachment #462749 - Flags: approval2.0? → approval2.0+
Keywords: checkin-needed
(Assignee)

Comment 6

8 years ago
http://hg.mozilla.org/mozilla-central/rev/28b218e903b0
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b4
You need to log in before you can comment on or make changes to this bug.