Donna Whisnant dewhisna@dewtronics.com writes:
Adds ARM ABI support to DispCallFunc() to allow IDispatch invoke calls to succeed on ARM platforms. This change specifically targets only 32-bit little-endian (ARMEL) platform CPUs. It's believed to likely be compatible with big-endian (ARMEB) platforms, but testing for that platform should be completed before enabling.
Tested on Raspbian Stretch 2017-09-07 RPi image using a Qemu systemd container to compile and run it on an x86-64 Host.. When running on Raspberry Pi hardware, a 3G/1G split kernel needs to be used for Wine to function.
It breaks the Android build:
arm-linux-androideabi-gcc -c -o typelib.o ../../../wine/dlls/oleaut32/typelib.c -I. -I../../../wine/dlls/oleaut32 \ -I../../include -I../../../wine/include -D__WINESRC__ -D_OLEAUT32_ -D_REENTRANT -fPIC -Wall -pipe \ -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers \ -Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wpointer-arith \ -Wlogical-op -gdwarf-2 -gstrict-dwarf -g -O2 -fno-diagnostics-show-caret -D__ANDROID_API__=26 -marm {standard input}: Assembler messages: {standard input}:32: Error: selected processor does not support ARM mode `vldm r3!,{s0-s15}' Makefile:516: recipe for target 'typelib.o' failed make: *** [typelib.o] Error 1
On Tue, 31 Oct 2017, Alexandre Julliard wrote:
Donna Whisnant dewhisna@dewtronics.com writes:
Adds ARM ABI support to DispCallFunc() to allow IDispatch invoke calls to succeed on ARM platforms. This change specifically targets only 32-bit little-endian (ARMEL) platform CPUs. It's believed to likely be compatible with big-endian (ARMEB) platforms, but testing for that platform should be completed before enabling.
Tested on Raspbian Stretch 2017-09-07 RPi image using a Qemu systemd container to compile and run it on an x86-64 Host.. When running on Raspberry Pi hardware, a 3G/1G split kernel needs to be used for Wine to function.
It breaks the Android build:
arm-linux-androideabi-gcc -c -o typelib.o ../../../wine/dlls/oleaut32/typelib.c -I. -I../../../wine/dlls/oleaut32 \ -I../../include -I../../../wine/include -D__WINESRC__ -D_OLEAUT32_ -D_REENTRANT -fPIC -Wall -pipe \ -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers \ -Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wpointer-arith \ -Wlogical-op -gdwarf-2 -gstrict-dwarf -g -O2 -fno-diagnostics-show-caret -D__ANDROID_API__=26 -marm {standard input}: Assembler messages: {standard input}:32: Error: selected processor does not support ARM mode `vldm r3!,{s0-s15}' Makefile:516: recipe for target 'typelib.o' failed make: *** [typelib.o] Error 1
FWIW, this is the same issue as was discussed before here: https://www.winehq.org/pipermail/wine-devel/2017-September/118872.html
// Martin
Ah... I hadn't tried it on a platform with soft-FP. And true enough, the compiler wouldn't support the VFP coprocessor instructions without hardware floats. It could be made to work with soft-FP too, but would at least be a different function for the assembly language part to one that excludes the VFP coprocessor instructions.
I think the correct answer for now would be to do the !defined(__SOFTFP__) in my code, just as described in the discussion at the link you sent. It seems correct since this code is explicitly written for hard-FP.
Please let me know if that's acceptable, and I will add that check and submit a v4 of the patch.
Also, another thing you will hit, that I mentioned in a previous discussion, is the test suite for this module. Currently, there is the following in tests/typelib.c:
static const BOOL is_win64 = sizeof(void *) > sizeof(int); ... /* the function checks the argument sizes for stdcall */ if (!is_win64) /* no stdcall on 64-bit */ { res = DispCallFunc( NULL, (ULONG_PTR)stdcall_func, CC_STDCALL, VT_UI4, 0, types, pargs, &result ); ok( res == DISP_E_BADCALLEE, "DispCallFunc wrong error %x\n", res ); res = DispCallFunc( NULL, (ULONG_PTR)stdcall_func, CC_STDCALL, VT_UI4, 1, types, pargs, &result ); ok( res == S_OK, "DispCallFunc failed %x\n", res ); res = DispCallFunc( NULL, (ULONG_PTR)stdcall_func, CC_STDCALL, VT_UI4, 2, types, pargs, &result ); ok( res == DISP_E_BADCALLEE, "DispCallFunc wrong error %x\n", res ); }
However, the stdcall tests have absolutely nothing to do with whether or not it's win64, but rather if the ABI itself supports stdcall or not. In my opinion, I believe there should be a constant defined at the top according to the ABI. I *think* the only platform that supports stdcall like that is i386, so have something like:
#ifdef (__i386__) static const BOOL abi_supports_stdcall = TRUE; #else static const BOOL abi_supports_stdcall = FALSE; #endif
And then change the 'if' using it to be: if (abi_supports_stdcall) { ... }
I hadn't submitted that patch yet, since I didn't know how you wanted to address it. But, if that's an acceptable solution, I would be glad to make a patch for the test suite too.
Donna
On Tue, Oct 31, 2017, at 05:02, Martin Storsjö wrote:
On Tue, 31 Oct 2017, Alexandre Julliard wrote:
Donna Whisnant dewhisna@dewtronics.com writes:
Adds ARM ABI support to DispCallFunc() to allow IDispatch invoke calls to succeed on ARM platforms. This change specifically targets only 32-bit little-endian (ARMEL) platform CPUs. It's believed to likely be compatible with big-endian (ARMEB) platforms, but testing for that platform should be completed before enabling.
Tested on Raspbian Stretch 2017-09-07 RPi image using a Qemu systemd container to compile and run it on an x86-64 Host.. When running on Raspberry Pi hardware, a 3G/1G split kernel needs to be used for Wine to function.
It breaks the Android build:
arm-linux-androideabi-gcc -c -o typelib.o ../../../wine/dlls/oleaut32/typelib.c -I. -I../../../wine/dlls/oleaut32 \ -I../../include -I../../../wine/include -D__WINESRC__ -D_OLEAUT32_ -D_REENTRANT -fPIC -Wall -pipe \ -fno-strict-aliasing -Wdeclaration-after-statement -Wempty-body -Wignored-qualifiers \ -Wstrict-prototypes -Wtype-limits -Wunused-but-set-parameter -Wvla -Wwrite-strings -Wpointer-arith \ -Wlogical-op -gdwarf-2 -gstrict-dwarf -g -O2 -fno-diagnostics-show-caret -D__ANDROID_API__=26 -marm {standard input}: Assembler messages: {standard input}:32: Error: selected processor does not support ARM mode `vldm r3!,{s0-s15}' Makefile:516: recipe for target 'typelib.o' failed make: *** [typelib.o] Error 1
FWIW, this is the same issue as was discussed before here: https://www.winehq.org/pipermail/wine-devel/2017-September/118872.html
// Martin
Donna Whisnant dewhisna@dewtronics.com writes:
Ah... I hadn't tried it on a platform with soft-FP. And true enough, the compiler wouldn't support the VFP coprocessor instructions without hardware floats. It could be made to work with soft-FP too, but would at least be a different function for the assembly language part to one that excludes the VFP coprocessor instructions.
I think the correct answer for now would be to do the !defined(__SOFTFP__) in my code, just as described in the discussion at the link you sent. It seems correct since this code is explicitly written for hard-FP.
I've fixed my Android toolchain and added a configure check to require floating point support, so hopefully this will work now.
However, the stdcall tests have absolutely nothing to do with whether or not it's win64, but rather if the ABI itself supports stdcall or not. In my opinion, I believe there should be a constant defined at the top according to the ABI. I *think* the only platform that supports stdcall like that is i386, so have something like:
#ifdef (__i386__) static const BOOL abi_supports_stdcall = TRUE; #else static const BOOL abi_supports_stdcall = FALSE; #endif
And then change the 'if' using it to be: if (abi_supports_stdcall) { ... }
I hadn't submitted that patch yet, since I didn't know how you wanted to address it. But, if that's an acceptable solution, I would be glad to make a patch for the test suite too.
Sure, that would be fine.
Ah... I hadn't tried it on a platform with soft-FP. And true enough, the compiler wouldn't support the VFP coprocessor instructions without hardware floats. It could be made to work with soft-FP too, but would at least be a different function for the assembly language part to one that excludes the VFP coprocessor instructions.
I think the correct answer for now would be to do the !defined(__SOFTFP__) in my code, just as described in the discussion at the link you sent. It seems correct since this code is explicitly written for hard-FP.
I've fixed my Android toolchain and added a configure check to require floating point support, so hopefully this will work now.
Is that the best long-term solution? Wouldn't there be users who might wish to use Wine with soft-FP on ARM? I just dug out my old Android NDK toolchain (one downloaded from android.com prebuilt) from the last time I was doing Android development (about four years ago) and dumped the compiler's built-in defines, and confirmed that it was built for soft-FP, at least as the default. So I could envision someone wanting Wine to work with soft-FP.
One example that comes to mind is someone writing an app where they wish to make certain that data round-trips scaling calculations across multiple platforms. For hard-FP, that's usually done with setting the FP coprocessor configuration for specific modes, such as how denormalization is handled and/or how infinity is processed. But, when interacting with a compound software application (where there's a main application with plug-in components coming from separate design groups), there are cases when changing the coprocessor settings would adversely affect the parent application, and so using soft-FP is often the solution.
In fact, the application I've been working on that prompted my adding this function for ARM to Wine in the first place may end up being such an application (depends on how easily our floating-point janitor functions port to ARM). So it seems like the best general solution is to have two sets of functions, one for hard-FP and one for soft-FP.
I would have to research the soft-FP ABI details to confirm, but I believe for the soft-FP path it's just a matter of rolling the VT_R4 and VT_R8 cases in with the VT_I4 and VT_I8, respectively, in DispCallFunc(), drop the floats from the reg_args structure passed to the assembly function, and drop the 'vldm' instruction in the assembly code.
It would just be a matter of deciding if the code looks cleaner to keep the two totally separate or to just have a few special-cases for the soft-FP mode. If you'd like, I can write the soft-FP case and submit it, and that would keep from backing us in a corner of only supporting hard-FP with Wine. It could allow Wine to be used on many other ARM type devices out there.
Donna
Donna Whisnant dewhisna@dewtronics.com writes:
It would just be a matter of deciding if the code looks cleaner to keep the two totally separate or to just have a few special-cases for the soft-FP mode. If you'd like, I can write the soft-FP case and submit it, and that would keep from backing us in a corner of only supporting hard-FP with Wine. It could allow Wine to be used on many other ARM type devices out there.
The thing is that we need hard-float for binary compatibility, which should be the default behavior. I'm not opposed to having a soft-float option that you could request explicitly for special cases, though without a demonstrated use case, I'm concerned that the code would just bit-rot.