Closed Bug 570882 Opened 14 years ago Closed 14 years ago

Need assembler optimization for VPX on Windows x64

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Platform:
x86_64
Windows Vista
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: m_kato, Assigned: m_kato)

References

Details

Attachments

(2 files)

patch
5.71 KB, patch
cpearce
: review+
Details | Diff | Splinter Review
typo fix
1.05 KB, patch
cpearce
: review+
Details | Diff | Splinter Review 
Currently assembler (MMX or SSE2) optimization is turned off on Windows x64 build.

I consider that...
- Use YASM on Windows x64
or
- Use shell script to use ML64.exe if we can do it
(In reply to comment #0)
> Currently assembler (MMX or SSE2) optimization is turned off on Windows x64
> build.

We won't even build on Win64, unless you have a stdint.h, which only exists on Windows in VS2010.

> - Use YASM on Windows x64

I couldn't get YASM to produce linkable object files on Win32 because YASM produces invalid SafeSeh code (see: http://www.tortall.net/projects/yasm/ticket/139 ), and we link libxul with /SAFESEH. I'm not sure if this YASM bug affects the Win64 assembly or not.

Long term, we'd ideally use YASM to compile the assembly, so that we're using the same tool on all platforms.

> - Use shell script to use ML64.exe if we can do it

This may be the easiest short term, until YASM's SafeSeh generator is fixed. 

You'll need to checkout libvpx, and configure a build target for x86_64-win64_vs8. I documented how to do this for Win32 here:

https://wiki.mozilla.org/WebM/Updating_libvpx
(In reply to comment #1)
> We won't even build on Win64, unless you have a stdint.h, which only exists on
> Windows in VS2010.

Actually, Tinderbox uses VS2010 on Windows x64 build.

> 
> > - Use YASM on Windows x64
> 
> I couldn't get YASM to produce linkable object files on Win32 because YASM
> produces invalid SafeSeh code (see:
> http://www.tortall.net/projects/yasm/ticket/139 ), and we link libxul with
> /SAFESEH. I'm not sure if this YASM bug affects the Win64 assembly or not.

safeseh is x86 only flag.  So I think that this isn't influence.

> Long term, we'd ideally use YASM to compile the assembly, so that we're using
> the same tool on all platforms.

I see.

> > - Use shell script to use ML64.exe if we can do it
> 
> This may be the easiest short term, until YASM's SafeSeh generator is fixed. 

ML64 doesn't support MMX, so I may have to change Makefile and code to use ML64.exe
(In reply to comment #2)
> Actually, Tinderbox uses VS2010 on Windows x64 build.

Excellent. We will at least build on Win64!

> ML64 doesn't support MMX, so I may have to change Makefile and code to use
> ML64.exe

I didn't realise that ML64 doesn't support MMX. If we can use YASM on Win64, I think we should. It will be easier than maintaining someone else's assembly code. We'd need to get YASM installed on the Win64 mozilla build machines (we installed it on the Mac and Linux build machines already).

Long term we want to use YASM everywhere, so we may as well use YASM on Win64 if we can. We're planning on adding YASM to MozillaBuild (bug 570477) to facilitate using YASM on Windows in the future.
Requiring and using YASM on Win64 sounds good to me.
We probably should just contribute a fix to that SAFESEH bug ourselves.
Attached patch patch — — Splinter Review 

    
  

        Attachment #450075 -
        Flags: review?(chris)

    
    

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

Excellent, thanks very much! We'll need to get YASM installed on the Win64 machines before we can land this.

        Attachment #450075 -
        Flags: review?(chris) → review+

    
    

    
  
landed.
http://hg.mozilla.org/mozilla-central/rev/993be74a2a7f

If yasm isn't installed, don't use asm optimize.

But even if don't use asm optimize, win64 build will be break.
See http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276077720.1276081955.15028.gz

    
    

    
  
As long as I check http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276077720.1276081955.15028.gz (I don't land this at this point yet), I recommend that we use --disable-webm before isntalling YASM.

Armen, should I file a bug for your work item?

    
    

    
  
Does bug 569812 affect this? It's about an issue with the Win64 ABI if we enable assembly optimisations on Win 64.

    
    

    
  
Hi,
The Windows 64-bit build fails with or without --disable-webm at this point.

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276082029.1276085939.12252.gz
(short log)
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276082029.1276085939.12252.gz&fulltext=1

After that I tried to used --disable-webm and I got the same problem:
# Disable until bug 570882 is fixed
ac_add_options --disable-webm

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276101974.1276106512.29449.gz
(short log)
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276101974.1276106512.29449.gz&fulltext=1

Is this what I want?
http://www.tortall.net/projects/yasm/releases/vsyasm-1.0.1-win64.zip
or this?
http://www.tortall.net/projects/yasm/releases/yasm-1.0.1-win64.exe

With the first package I don't know where to put the files since there are two places where it could be placed:
> C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin
or
> C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\BuildCustomizations

The executable starts and finished and therefore I don't know if it got installed properly.

Could you please indicate me what is the right way of having this installed?

    
    

    
  
Sorry Armen, but --disable-webm is broken (discovered in bug 566247 comment 35). My bad, will fix today. :(

It shouldn't matter where you put YASM, so long as it's in the MozillaBuild path.

The readme.txt in the yasm download zip bundle say to put yasm here:
C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin

    
  
Blocks: tracking_win64

    
    

    
  
(In reply to comment #12)
> 
> The readme.txt in the yasm download zip bundle say to put yasm here:
> C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin
>
I have deployed the executable and the other 3 files in there.
I have triggered another build; let's see how it goes

    
    

    
  
(In reply to comment #13)
> (In reply to comment #12)
> > 
> > The readme.txt in the yasm download zip bundle say to put yasm here:
> > C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin
> >
> I have deployed the executable and the other 3 files in there.
> I have triggered another build; let's see how it goes
OK this does not seem to have worked.
I have put the mozconfig back to what it was and have left those 4 files in the aforementioned location.

Please let me know what the fix is and if someone can verify that it works I will then get my hands on the slave or the mozconfigs.

    
    

    
  
Is yasm in the path on the build machines? Do you have an error log we can look at?

    
    

    
  
(In reply to comment #14)
> > > The readme.txt in the yasm download zip bundle say to put yasm here:
> > > C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin
Do I actually have to add this into the PATH?
Could I put the yasm.exe in another location if I need to?

Here is a full log:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1276165058.1276169455.27421.gz&fulltext=1

For reference, the current PATH during "make -f client.mk profiledbuild" is:
  PATH=D:\mozilla-build\msys\local\bin;d:\mozilla-build\wget;d:\mozilla-build\7zip;d:\mozilla-build\blat261\full;d:\mozilla-build\python25;d:\mozilla-build\svn-win32-1.6.3\bin;d:\mozilla-build\upx203w;d:\mozilla-build\emacs-22.3\bin;d:\mozilla-build\info-zip;d:\mozilla-build\nsis-2.22;d:\mozilla-build\nsis-2.33u;d:\mozilla-build\hg;d:\mozilla-build\python25\Scripts;d:\mozilla-build\kdiff3;d:\mozilla-build\vim\vim72;.;D:\mozilla-build\msys\local\bin;D:\mozilla-build\msys\mingw\bin;D:\mozilla-build\msys\bin;c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\BIN\amd64;c:\Windows\Microsoft.NET\Framework64\v4.0.30319;c:\Windows\Microsoft.NET\Framework64\v3.5;c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\VCPackages;c:\Program Files (x86)\Microsoft Visual Studio 10.0\Common7\IDE;c:\Program Files (x86)\Microsoft Visual Studio 10.0\Common7\Tools;c:\Program Files (x86)\HTML Help Workshop;c:\Program Files (x86)\Microsoft SDKs\Windows\v7.0A\bin\NETFX 4.0 Tools\x64;c:\Program Files (x86)\Microsoft SDKs\Windows\v7.0A\bin\x64;c:\Program Files (x86)\Microsoft SDKs\Windows\v7.0A\bin;c:\Windows\System32;c:\Windows;c:\Windows\System32\Wbem;d:\mozilla-build\moztools-x64\bin
Blocks: 571143

    
    

    
  
The build is not finding yasm, excerpt from the build log:

checking for YASM assembler... checking for yasm... no
/e/builds/moz2_slave/mozilla-central-win64-nightly/build/configure: line 17421: dhl: command not found
configure: warning: No assembler or assembly support for libvpx, using unoptimized C routines.

Based on your path above, it looks like yasm won't be in the path if it's in C:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\bin. 

yasm must be in the path. yasm is executed by the make process by executing `yasm`, so if it's not in the path, it won't be found, and won't be executed. You can either:
1. Move yasm into the path (e.g. move it to D:\mozilla-build\msys\local\bin or perhaps c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\BIN\amd64), or
2. Add yasm's current location/directory to the path.

    
    

    
  
(In reply to comment #17)
> 1. Move yasm into the path (e.g. move it to D:\mozilla-build\msys\local\bin
I have done this and I had to rename vsyasm.exe as yasm.exe

I have triggered a build and I will let you know.

BTW are the other 3 files necessary for us?

    
    

    
  
(In reply to comment #18)
> BTW are the other 3 files necessary for us?

Probably not, I think they're related visual studio IDE integration.

    
    

    
  
The builds are green again.

I followed these steps:
* wget http://www.tortall.net/projects/yasm/releases/vsyasm-1.0.1-win64.zip
* unzip vsyasm-1.0.1-win64.zip
* mv vsyasm.exe /d/mozilla-build/msys/bin/yasm.exe

I have asked in bug 570473 to add yasm.exe into MozillaBuild 1.5. Not sure if that will be possible or the right way to get it to people.

    
    

    
  
(In reply to comment #21)
> Looks like dhl vs. dnl got landed on comm-central
> http://mxr.mozilla.org/comm-central/source/mozilla/configure.in#5924

Actually, that's mozilla-central's configure.in ;-)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        typo fix
        — 
          — Splinter Review
      

    


  

    
    

    
  
Comment on attachment 451543 [details] [diff] [review]
typo fix

Typo fix.

Do I need review this?

        Attachment #451543 -
        Flags: review?(chris)

    
    

    
  
Hi,

thank you for fixing the dhl typo. This affected comm-central TB3 build.

Now, it seems that you guys know something about webm feature.

Would you like to modify the comment regarding alsa in the configure.in as follows?
(Or should I file a separate bug?)

That is, add a comment that --disable-webm is necessary when alsa (the
audio module under linux) is not in the system.

I posted this to mozilla newsgroup, mozilla.dev.builds :

TIA
---
Environment: linux

While trying to compile TB3 (the developer version (nightly)) using
comm-central, I noticed a configuration failure.

One reason is that there is a typo of "dhl" which should have read "dnl" in
configure.in, and this is a problem of configure.in and needs to be fixed.

The other reason is that I missed alsa package (under linux) according to
configure message.
However, this machine doesn't have audio and I have not
bothered to load audio related package, and
I have explicitly disabled wave and ogg
with

ac_add_options --disable-ogg
ac_add_options --disable-wave

in MOZCONFIG file and successfully built TB3 before.

It turns out that the latest comm-central code requires
the additional disabling of webm feature by the  following:

ac_add_options --disable-webm

MOZ_SYDNEYAUDIO, whatever it stands for, gets set if webm feature is used,
and this MOZ_SYDNEYAUDIO is checked to see if alsa package is needed.


I have modified mozilla/configure.in as follows:
(Not sure if the patch goes out straight without unintended line-break.)
At least now the configure succeeds.


diff -r 57e341802e7d configure.in
--- a/configure.in	Tue Jun 15 23:55:46 2010 -0400
+++ b/configure.in	Wed Jun 16 17:33:45 2010 +0900
@@ -5916,17 +5916,17 @@
           VPX_X86_ASM=1
           VPX_AS='sh $(topsrcdir)/media/libvpx/yasm2masm-as.sh'
         else
             AC_MSG_ERROR([Need MASM (ml.exe) in order to assemble libvpx
optimized assembly. Either disable webm (reconfigure with --disable-webm) or
install MASM. MASM is included in the Windows 7 SDK, or you can download
MASM directly from
http://www.microsoft.com/downloads/details.aspx?familyid=7a1c9da0-0510-44a2-b042-7ef370530c64&displaylang=en])
         fi
     else

       dnl For Darwin x86, Darwin x86_64, Linux x86, and WINNT x86_64
-      dhl we can use YASM.
+      dnl we can use YASM.
       AC_MSG_CHECKING([for YASM assembler])
       AC_CHECK_PROGS(VPX_AS, yasm, "")
       if test -n "$VPX_AS"; then
         dnl We have YASM, see if we have assembly on this platform.
         case "$OS_ARCH:$OS_TEST" in
         Linux:x86|Linux:i?86)
           VPX_ASFLAGS="-f elf32 -rnasm -pnasm"
           VPX_X86_ASM=1
@@ -5989,17 +5989,17 @@
 dnl ========================================================

 dnl If using sydneyaudio with Linux, ensure that the alsa library is available
 if test -n "$MOZ_SYDNEYAUDIO"; then
    case "$target_os" in
 linux*)
       PKG_CHECK_MODULES(MOZ_ALSA, alsa, ,
          [echo "$MOZ_ALSA_PKG_ERRORS"
-          AC_MSG_ERROR([Need alsa for Ogg or Wave decoding on Linux.
Disable with --disable-ogg --disable-wave.  (On Ubuntu, you might try
installing the package libasound2-dev.)])])
+          AC_MSG_ERROR([Need alsa for Ogg or Wave decoding on Linux.
Disable with --disable-ogg --disable-wave --disable=webm.  (On Ubuntu, you
might try installing the package libasound2-dev.)])])
       ;;
    esac
 fi

 dnl ========================================================
 dnl Splashscreen
 dnl ========================================================
 AC_ARG_ENABLE(splashscreen,


FYI

TIA

    
    

    
  
I am not sure now whether the mere comment modification is good enough or not.
Please see:
https://bugzilla.mozilla.org/show_bug.cgi?id=572001

    
    

    
  
Comment on attachment 451543 [details] [diff] [review]
typo fix

Thanks!

        Attachment #451543 -
        Flags: review?(chris) → review+

    
    

    
  
(In reply to comment #25)
> Would you like to modify the comment regarding alsa in the configure.in as
> follows?
> (Or should I file a separate bug?)

Changing the ALSA error message to suggest --disable-webm is fine. Could you file a separate bug in Core -> Video/Audio with your patch please?
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED

    
    

    
  
(In reply to comment #28)
> (In reply to comment #25)
> > Would you like to modify the comment regarding alsa in the configure.in as
> > follows?
> > (Or should I file a separate bug?)
> 
> Changing the ALSA error message to suggest --disable-webm is fine. Could you
> file a separate bug in Core -> Video/Audio with your patch please?

Thank you for your comment.

I filed 
https://bugzilla.mozilla.org/show_bug.cgi?id=572635
Bug 572635  - Configure needs to have --disable-webm for non-audio environment besides --disable-wave and --disable-ogg

TIA

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: