Include 64-bit optimized assembly on Windows when building x64

Include 64-bit optimized assembly on Windows when building x64

BUG=167951
TEST='nss' compiles and links
R=wtc@chromium.org

[Ryan Sleevi, 2013-01-02 05:57:56]

wtc: NSS #define rules
scottmg: What's your feel on these MASM changes being here? The alternative
seems to be adding it GYP & msvs_emulation.py, but then it's no longer emulating
MSVS's behaviour (which is to auto-masm only in x32, not x64).

That said, MSBuild 2010+'s masm target does properly handle both masm.exe and
ml64.exe rules contingent upon target arch, so there is somewhat of a case to be
made for having GYP handle this.

[scottmg, 2013-01-02 16:01:25]

On 2013/01/02 05:57:56, Ryan Sleevi wrote:
> wtc: NSS #define rules
> scottmg: What's your feel on these MASM changes being here? The alternative
> seems to be adding it GYP & msvs_emulation.py, but then it's no longer
emulating
> MSVS's behaviour (which is to auto-masm only in x32, not x64).
> 
> That said, MSBuild 2010+'s masm target does properly handle both masm.exe and
> ml64.exe rules contingent upon target arch, so there is somewhat of a case to
be
> made for having GYP handle this.

It seems reasonable to add it to gyp, even though it doesn't work <= 2008. It'd
probably be better since presumably there will be more places where we need ml64
to get x64 up.

That said, I don't really know what the .asm situation looks like globally. e.g.
Are there cases where we'll need to build a ".asm" as x86 even in the x64 build?

[wtc, 2013-01-04 16:26:40]

Patch set 2 LGTM.

https://codereview.chromium.org/11738002/diff/2002/nss.gyp
File nss.gyp (right):

https://codereview.chromium.org/11738002/diff/2002/nss.gyp#newcode1129
nss.gyp:1129: 'WIN64',

coreconf/WIN32.mk also defines _AMD64_. It may not be used,
but it is best to match that faithfully.

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/WIN32.m...

https://codereview.chromium.org/11738002/diff/2002/nss.gyp#newcode1132
nss.gyp:1132: 'MP_IS_LITTLE_ENDIAN',

Not sure if you meant to sort these macros alphabetically or
match the order in which they're defined in the NSS makefile:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/freebl/M...

Note: I generated the macro definitions in this .gyp file
by examining the compiler command lines of an upstream NSS
build. It would be nice to repeat that for an upstream NSS
WIN64 build, but I believe it should be sufficient to just
examine coreconf/WIN32.mk and lib/freebl/Makefile.

https://codereview.chromium.org/11738002/diff/2002/nss.gyp#newcode1142
nss.gyp:1142: 'output_file': '<(INTERMEDIATE_DIR)/<(RULE_INPUT_ROOT)_asm.obj'

We should not need _asm before .obj. The NSS coreconf build
system does the following:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/WIN32.m...

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/ruleset...

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/rules.m...

https://codereview.chromium.org/11738002/diff/2002/nss.gyp#newcode1151
nss.gyp:1151: '/Ta', '<(RULE_INPUT_PATH)',

The NSS coreconf build system passes some flags to ml64.exe:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/WIN32.m...

Please check if we need any of those flags here.

https://codereview.chromium.org/11738002/diff/2002/nss.gyp#newcode1165
nss.gyp:1165: }, {

A comment here would be useful because this "else" block is
very far from the corresponding 'OS=="win"' condition.

https://codereview.chromium.org/11738002/diff/2002/scripts/nss-checkout.sh
File scripts/nss-checkout.sh (right):

https://codereview.chromium.org/11738002/diff/2002/scripts/nss-checkout.sh#ne...
scripts/nss-checkout.sh:78: ! -name mpi_amd64_masm.asm ! -name
mp_comba_amd64_masm.asm \

mp_comba_amd64_masm.asm should be listed with the other
mp_ files (as opposed to mpi_ files).

[Ryan Sleevi, 2013-01-07 03:32:59]

On 2013/01/02 16:01:25, scottmg wrote:
> On 2013/01/02 05:57:56, Ryan Sleevi wrote:
> > wtc: NSS #define rules
> > scottmg: What's your feel on these MASM changes being here? The alternative
> > seems to be adding it GYP & msvs_emulation.py, but then it's no longer
> emulating
> > MSVS's behaviour (which is to auto-masm only in x32, not x64).
> > 
> > That said, MSBuild 2010+'s masm target does properly handle both masm.exe
and
> > ml64.exe rules contingent upon target arch, so there is somewhat of a case
to
> be
> > made for having GYP handle this.
> 
> It seems reasonable to add it to gyp, even though it doesn't work <= 2008.
It'd
> probably be better since presumably there will be more places where we need
ml64
> to get x64 up.
> 
> That said, I don't really know what the .asm situation looks like globally.
e.g.
> Are there cases where we'll need to build a ".asm" as x86 even in the x64
build?

Not really. The vast majority of the code uses YASM directly. NSS is the only
one (AFAICT) that uses MASM. The alternative to adding it to GYP would be to
just convert NSS to YASM - something that I think upstream would accept, given
that Mozilla now requires YASM.

[Ryan Sleevi, 2013-01-07 03:37:34]

I have https://codereview.chromium.org/11742015/ pending to add support for MASM
to GYP & Winja.

An alternative here would be to convert the existing MASM files to YASM,
upstream. MozillaBuild (their Windows build environment, needed for NSS)
includes YASM since late 2010 (1.5.1+), as a result of Mozilla include
libjpeg_turbo and libvpx. This may be a better long-term alternative for
upstream NSS, since the YASM syntax is more robust and, arguably, widely
supported.

https://codereview.chromium.org/11738002/diff/2002/nss.gyp
File nss.gyp (right):

https://codereview.chromium.org/11738002/diff/2002/nss.gyp#newcode1132
nss.gyp:1132: 'MP_IS_LITTLE_ENDIAN',
On 2013/01/04 16:26:40, wtc wrote:
> 
> Not sure if you meant to sort these macros alphabetically or
> match the order in which they're defined in the NSS makefile:
>
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/freebl/M...

To match the order

> 
> Note: I generated the macro definitions in this .gyp file
> by examining the compiler command lines of an upstream NSS
> build. It would be nice to repeat that for an upstream NSS
> WIN64 build, but I believe it should be sufficient to just
> examine coreconf/WIN32.mk and lib/freebl/Makefile.

I'll confirm. I focused on lib/freebl/Makefile, and forgot to check
coreconf/WIN32.mk

https://codereview.chromium.org/11738002/diff/2002/nss.gyp#newcode1142
nss.gyp:1142: 'output_file': '<(INTERMEDIATE_DIR)/<(RULE_INPUT_ROOT)_asm.obj'
On 2013/01/04 16:26:40, wtc wrote:
> 
> We should not need _asm before .obj. The NSS coreconf build
> system does the following:
> 
>
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/WIN32.m...
> 
>
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/ruleset...
> 
>
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/rules.m...

There is no need to match what NSS does here for the output file naming. This
matches how we output YASM files, to avoid name conflicts (which your examples
show NSS is doing upstream, effectively)

https://codereview.chromium.org/11738002/diff/2002/nss.gyp#newcode1151
nss.gyp:1151: '/Ta', '<(RULE_INPUT_PATH)',
On 2013/01/04 16:26:40, wtc wrote:
> 
> The NSS coreconf build system passes some flags to ml64.exe:
>
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/coreconf/WIN32.m...
> 
> Please check if we need any of those flags here.

Yes, it appears these flags are tied to legacy MASM behaviours. Since we're
talking 2010+ here, it's equivalent to omit the flags.

[scottmg, 2013-01-07 16:20:54]

On 2013/01/07 03:37:34, Ryan Sleevi wrote:
> I have https://codereview.chromium.org/11742015/ pending to add support for
MASM
> to GYP & Winja.
> 
> An alternative here would be to convert the existing MASM files to YASM,
> upstream. MozillaBuild (their Windows build environment, needed for NSS)
> includes YASM since late 2010 (1.5.1+), as a result of Mozilla include
> libjpeg_turbo and libvpx. This may be a better long-term alternative for
> upstream NSS, since the YASM syntax is more robust and, arguably, widely
> supported.

If we could homogenize to everything using yasm, it seems like that would be a
good thing. I don't see any downside to masm x64 support in gyp, so up to you
whether you want to pursue pushing yasm to upstream at this point.

[wtc, 2013-01-10 03:21:09]

https://codereview.chromium.org/11738002/diff/2002/nss.gyp
File nss.gyp (right):

https://codereview.chromium.org/11738002/diff/2002/nss.gyp#newcode1132
nss.gyp:1132: 'MP_IS_LITTLE_ENDIAN',

On 2013/01/07 03:37:34, Ryan Sleevi wrote:
> On 2013/01/04 16:26:40, wtc wrote:
> > 
> > Not sure if you meant to sort these macros alphabetically or
> > match the order in which they're defined in the NSS makefile:
> >
>
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/freebl/M...
> 
> To match the order

These macros are defined in lib/freebl/Makefile in this
order:

  MP_CHAR_STORE_SLOW
  MP_IS_LITTLE_ENDIAN
  NSS_BEVAND_ARCFOUR
  MPI_AMD64
  MP_ASSEMBLY_MULTIPLY
  NSS_USE_COMBA

The order used in this file is not sorted (but very
close) and doesn't match the order used in
lib/freebl/Makefile.

https://codereview.chromium.org/11738002/diff/2002/nss.gyp#newcode1142
nss.gyp:1142: 'output_file': '<(INTERMEDIATE_DIR)/<(RULE_INPUT_ROOT)_asm.obj'

Names of assembly code files typically contain the processor
or instruction set architecture names, so name conflicts with
C/C++ file names should be unlikely. (NSS does have a near
conflict: mpi_amd64.c and mpi_amd64_masm.asm.) In addition,
having the source files foo.c and foo.s in the same
directory is problematic if makefiles are used.

Can we remove the "_asm" from the GYP file rule for YASM
files? I wonder if it is a solution to a non-problem.
If there is a name conflict in the Chromium source tree,
I think it is better to simply append .obj to the original
assembly file name (including the .asm extension).