572876 - Enable VP8 asm on x86-64 Linux

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Timothy B. Terriberry (:derf)]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.8.1.23) Gecko/20100611 SeaMonkey/1.1.18
Build Identifier: 

Asm for VP8 was previously disabled on x86-64 Linux due to linking problems. These problems should be resolved and asm enabled on this platform.

Reproducible: Always

Comment 1

[Timothy B. Terriberry (:derf)]

Attachment: Patch to fix the linker issue on x86-64 Linux and enable VP8 asm.

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 452084 [details] [diff] [review]
Patch to fix the linker issue on x86-64 Linux and enable VP8 asm.

The ":data hidden" directive breaks the yasm2masm.py translator/ml.exe wrapper:

YASM-to-MASM translate-compile: /d/work/src/purple/media/libvpx/vp8/common/x86/subpixel_mmx.asm
ml -c -Fo subpixel_mmx.obj  -I/d/work/src/purple/media/libvpx/ -I/d/work/src/purple/media/libvpx/vpx_ports/ -D __OUTPUT_FORMAT__=win32 -safeseh subpixel_mmx.obj
.p
Microsoft (R) Macro Assembler Version 9.00.30729.01
Copyright (C) Microsoft Corporation.  All rights reserved.

 Assembling: subpixel_mmx.obj.p
subpixel_mmx.obj.p(735) : error A2008:syntax error : :
subpixel_mmx.obj.p(795) : error A2008:syntax error : :
make[1]: *** [subpixel_mmx.obj] Error 1

Can you make the ":data hidden" into a preprocessor macro which only has a body on x86_64 Linux? Or find some other way to only have it present on x86_64 Linux? Thanks!

Comment 3

[Timothy B. Terriberry (:derf)]

Attachment: Patch to fix the linker issue on x86-64 Linux and enable VP8 asm.

This version uses a macro to only apply the :data hidden directive for ELF64 targets.

Comment 4

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 452099 [details] [diff] [review]
Patch to fix the linker issue on x86-64 Linux and enable VP8 asm.

I've pushed to tryserver, and have also started x86 linux, mac, and win32 builds locally with this patch applied. You need to add the following to your patch, to compile on Win32. If you add that, and it builds on the platforms I'm testing, I'm happy to r+ it.

diff --git a/media/libvpx/vpx_ports/x86_abi_support_win32.asm b/media/libvpx/vpx_ports/x86_abi_support_win32.asm
--- a/media/libvpx/vpx_ports/x86_abi_support_win32.asm
+++ b/media/libvpx/vpx_ports/x86_abi_support_win32.asm
@@ -61,13 +61,15 @@ SHADOW_ARGS_TO_STACK macro v
                      endm
 UNSHADOW_ARGS macro v
               endm

 SECTION_RODATA macro
                .const
                endm

+HIDDEN_DATA textequ <>
+
 .686p
 .XMM
 .model flat, C
 option casemap :none    ; be case-insensitive
 .code

Comment 5

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to comment #4)
> (From update of attachment 452099 [details] [diff] [review])
> I've pushed to tryserver, and have also started x86 linux, mac, and win32
> builds locally with this patch applied. 

Local x86 Linux and Win32 builds succeeded (with the addition from comment 4). Mac and TryServer will take longer, to complete.

Comment 6

[Timothy B. Terriberry (:derf)]

Attachment: Patch to fix the linker issue on x86-64 Linux and enable VP8 asm.

Add HIDDEN_DATA macro for masm win32 build.

Comment 7

[Makoto Kato [:m_kato]]

Add bug URL on Issue tracker of libvpx

Comment 8

[Timothy B. Terriberry (:derf)]

This has been submitted upstream as
http://review.webmproject.org/187

Comment 9

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/2f0443835b3d