687033 - add Windows/arm target (NSPR part)

Status: ASSIGNED

(NSPR :: NSPR, enhancement)

Description

[Makoto Kato [:m_kato]]

Attachment: WIP

VS11 (VS2011? or VS2012?) supports ARM. But, although VS11 dev preview is still broken (ex. lack of some libraries such as wsock32, winmm and etc), I would like to add arm target if VS11 dp/ctp/beta is refreshed.

To add arm target, we must add cross-compiling environment (target=arm, host=x86) for MSVC only.

Comment 1

[Jim Mathies [:jimm]]

I've been doing some experimentation with this in the hope of getting a jail broken Surface RT running both desktop and metro fx. A few notes thus far:

1) CFLAGS and CXXFLAGS, including the defines used for AC_TRY_COMPILE need a define to get past a compiler directive in crtdefs.h - 

-D_ARM_WINAPI_PARTITION_DESKTOP_SDK_AVAILABLE=1

2) Add the proper MACHINE linker option to DSO_LDOPTS:

-MACHINE:ARM

3) There's a configure check that fails which I haven't had the time to sort out so I simply removed it for experimentation:

AC_MSG_CHECKING([for posix_fallocate]) (delete or updated)

4) Modify an MSBuild arm target config file as well -

C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V110\Platforms\ARM\Microsoft.Cpp.ARM.Common.props

add the following entry to PropertyGroup:

<WindowsSDKDesktopARMSupport>true</WindowsSDKDesktopARMSupport>

5) Add various arm paths to your LIB, LIBPATH, and PATH variables to give the arm tools available precedence -

Microsoft Visual Studio 11.0\VC\LIB\arm
Microsoft Visual Studio 11.0\VC\atlmfc\lib\arm
Windows Kits\8.0\Lib\win8\um\arm
Microsoft Visual Studio 11.0/VC/BIN/x86_ARM
Windows Kits/8.0/bin/arm

This will get you most of the way through pymake configure. I'm currently running into problems with config in the 3rd party library libffi which doesn't support a Windows arm target. Looks like this is related to ctypes. We'll probably have to disable this library to get past it. Not sure if we support working without it though.

Comment 2

[Makoto Kato [:m_kato]]

Actually, nsprpub and js.exe works on my jailbreak's SurfaceRT.

Additional, all configuration for arm needs -D_CRT_BUILD_APP=0 to use std*.h

Comment 3

[Jim Mathies [:jimm]]

(In reply to Makoto Kato from comment #2)
> Actually, nsprpub and js.exe works on my jailbreak's SurfaceRT.
> 
> Additional, all configuration for arm needs -D_CRT_BUILD_APP=0 to use std*.h

sweet! :) I'm up to nspr in my build and am failing. Are you building nspr with the changes you posted here?

Comment 4

[Jim Mathies [:jimm]]

A couple additional notes, I had to disable ctypes - 

ac_add_options --disable-system-ffi
ac_add_options --disable-ctypes

and mfbt needed an update - 

diff --git a/mfbt/double-conversion/utils.h b/mfbt/double-conversion/utils.h
--- a/mfbt/double-conversion/utils.h
+++ b/mfbt/double-conversion/utils.h
@@ -62,16 +62,19 @@
 #define DOUBLE_CONVERSION_CORRECT_DOUBLE_OPERATIONS 1
 #elif defined(_M_IX86) || defined(__i386__) || defined(__i386)
 #if defined(_WIN32)
 // Windows uses a 64bit wide floating point stack.
 #define DOUBLE_CONVERSION_CORRECT_DOUBLE_OPERATIONS 1
 #else
 #undef DOUBLE_CONVERSION_CORRECT_DOUBLE_OPERATIONS
 #endif  // _WIN32
+#elif defined(__arm__)
+// XXX Not sure what the width of the floating point stack is for Windows ARM
+#undef DOUBLE_CONVERSION_CORRECT_DOUBLE_OPERATIONS
 #else
 #error Target architecture was not detected as supported by Double-Conversion.
 #endif

and I had to add __arm__ to the default defines in configure. Apparently the sdk headers add _ARM_, but there is no built in __arm__ define.

Comment 5

[Makoto Kato [:m_kato]]

__arm__ isn't used on cl.exe. Microsoft compiler uses  _M_ARM for arm target

Comment 6

[Makoto Kato [:m_kato]]

I will update patches for reviewable.

Comment 7

[Makoto Kato [:m_kato]]

Attachment: Part 1: support Win32/arm (code part1)

Comment 8

[Makoto Kato [:m_kato]]

Attachment: Part 2: build config part

This patch includes bug 759239.

Comment 9

[Makoto Kato [:m_kato]]

Attachment: Part 2: build config part

Comment 10

[Makoto Kato [:m_kato]]

Comment on attachment 710044 [details] [diff] [review]
Part 1: support Win32/arm (code part1)

This fix is for jailbreak arm target.

If we need store API only, we have to fix bug 746441 and etc.

Comment 11

[Makoto Kato [:m_kato]]

Comment on attachment 710046 [details] [diff] [review]
Part 2: build config part

This fix includes bug 759239.  (To cross building using MSVS, I need bug 759239's fix)

If landing bug 759239, I will create a patch again.

Comment 12

[Makoto Kato [:m_kato]]

hwo to build.

my nspr path: c:/Development/cvs-mirror.mozilla.org/winrt/
VS2012 path: <VS2012 PATH>

HOST_CC=cl CC=<VS2012 PATH>/VC/BIN/X86_ARM/CL.EXE LD=link  DLLFLAGS="-LIBPATH:\"C:/Program Files (x86)/Windows Kits/8.0/LIB/WIN8/UM/ARM\" -LIBPATH:\"<VS2012 PATH>/VC/LIB/ARM\"" /c/Development/cvs-mirror.mozilla.org/winrt/mozilla/nsprpub/configure  --disable-debug --disable-tests --target=arm-pc-mingw32 --with-dist-prefix=c:/Development/cvs-mirror.mozilla.org/winrt/objdir/dist --with-mozilla --enable-optimize --srcdir=/c/Development/cvs-mirror.mozilla.org/winrt/mozilla/nsprpub/

Comment 13

[Masatoshi Kimura [:emk]]

Comment on attachment 710044 [details] [diff] [review]
Part 1: support Win32/arm (code part1)

>-/* Windows CE has GetFileAttributesEx. */
>-#ifndef WINCE
>+/* Windows CE and Windows on x64 and arm have GetFileAttributesEx. */
>+#if !defined(WINCE) && !defined(_M_ARM) && !defined(_M_X64)

I think we can just assume GetFileAttributesEx is always available.
I was told by wtc that the current NSPR system requirement is XPSP2.

Comment 14

[Wan-Teh Chang]

Comment on attachment 710044 [details] [diff] [review]
Part 1: support Win32/arm (code part1)

Makoto: thank you for the patch. The changes in this patch seem
very small. So even though I question the need for this patch,
I am probably willing to check this in.

Could you take the opportunity to delete the WINCE code?
Just delete the WINCE code if you need to modify it. (I'm not
asking you to delete all the WINCE code in NSPR.)

Comment 15

[Wan-Teh Chang]

Comment on attachment 710046 [details] [diff] [review]
Part 2: build config part

This patch also seems small, but I will ask Ted to review it
because I am not familiar with cross-compiling NSPR.

Ted: please feel free to decline the review request because
right now this port seems like a "hobbyist" activity.

Comment 16

[Wan-Teh Chang]

Attachment: prsystem.c patch: assume GlobalMemoryStatusEx is available

NSPR only needs to support Windows XP and later now.
There is a comment in prsystem.c that says
GlobalMemoryStatusEx is available on Windows 2000+.
So we can delete the dynamic lookup code.

Comment 17

[Wan-Teh Chang]

Attachment: w95io.c patch: remove WINCE and Win9x support

This patch removes a lot of obsolete code from w95io.c.
We can assume GetFileAttributesEx is available. A comment
in w95io.c says "GetFileAttributesEx is supported on Win 2K
and up."

Also, this MXR query shows _pr_useUnicode is set but not
used, so we can remove the _pr_useUnicode global variable
altogether.

Comment 18

[Wan-Teh Chang]

Attachment: Part 1: support Win32/arm, v2, by Makoto Kato

We don't need to support Windows/arm in the deprecated
"WINNT" build configuration. So I removed the _winnt.cfg
and _winnt.h changes from your patch.

I addressed prsystem.c and w95io.c separately, inspired
by your patch. This patch is what remains.

Comment 19

[Wan-Teh Chang]

Comment on attachment 714173 [details] [diff] [review]
prsystem.c patch: assume GlobalMemoryStatusEx is available

https://hg.mozilla.org/projects/nspr/rev/9e3594ac2353

Comment 20

[Wan-Teh Chang]

Comment on attachment 714174 [details] [diff] [review]
w95io.c patch: remove WINCE and Win9x support

https://hg.mozilla.org/projects/nspr/rev/141140050d0e

Comment 21

[Wan-Teh Chang]

Comment on attachment 714179 [details] [diff] [review]
Part 1: support Win32/arm, v2, by Makoto Kato

https://hg.mozilla.org/projects/nspr/rev/0c076527d529

Comment 22

[(not currently active) Ted Mielczarek]

Comment on attachment 710046 [details] [diff] [review]
Part 2: build config part

Review of attachment 710046 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry this took so long. This looks fine.

::: configure.in
@@ +66,5 @@
>  HOST_LDFLAGS="${HOST_LDFLAGS=}"
>  
>  case "$target" in
> +arm-*-mingw*)
> +    dnl Win32/arm is corss target only

spelling: "cross"

@@ +1109,5 @@
>      thumb_flag="-marm"
>      ;;
>  *)
> +    if test "$GNU_CC"; then
> +        dnl __thumb2__ is gcc only

Does the Microsoft arm toolchain support generating thumb2 code? If so you should look into supporting that.