Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for Windows on Arm64 (WoA) build #413

Closed
wants to merge 2 commits into from
Closed

Add support for Windows on Arm64 (WoA) build #413

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a03aff7
Add support for Windows on Arm64 (WoA) build
chirontt Sep 27, 2022
94d36db
Revert custom dark mode code to default in WoA
chirontt Sep 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions bundles/org.eclipse.swt/.classpath_win32_aarch64
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?xml version="1.0" encoding="UTF-8"?>
<classpath>
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-11"/>
<classpathentry kind="src" path="Eclipse SWT/win32"/>
<classpathentry kind="src" path="Eclipse SWT/common"/>
<classpathentry kind="src" path="Eclipse SWT PI/common"/>
<classpathentry kind="src" path="Eclipse SWT PI/win32">
<attributes>
<attribute value="org.eclipse.swt.win32.win32.aarch64" name="org.eclipse.jdt.launching.CLASSPATH_ATTR_LIBRARY_PATH_ENTRY"/>
</attributes>
</classpathentry>
<classpathentry kind="src" path="Eclipse SWT OLE Win32/win32"/>
<classpathentry kind="src" path="Eclipse SWT Accessibility/win32"/>
<classpathentry kind="src" path="Eclipse SWT Accessibility/common"/>
<classpathentry kind="src" path="Eclipse SWT AWT/win32"/>
<classpathentry kind="src" path="Eclipse SWT AWT/common"/>
<classpathentry kind="src" path="Eclipse SWT Drag and Drop/win32"/>
<classpathentry kind="src" path="Eclipse SWT Drag and Drop/common"/>
<classpathentry kind="src" path="Eclipse SWT Printing/win32"/>
<classpathentry kind="src" path="Eclipse SWT Printing/common"/>
<classpathentry kind="src" path="Eclipse SWT Program/win32"/>
<classpathentry kind="src" path="Eclipse SWT Program/common"/>
<classpathentry kind="src" path="Eclipse SWT Custom Widgets/common"/>
<classpathentry kind="src" path="Eclipse SWT Browser/common"/>
<classpathentry kind="src" path="Eclipse SWT Browser/win32"/>
<classpathentry kind="src" path="Eclipse SWT OpenGL/win32"/>
<classpathentry kind="src" path="Eclipse SWT OpenGL/common"/>
<classpathentry kind="output" path="bin"/>
</classpath>
26 changes: 15 additions & 11 deletions bundles/org.eclipse.swt/Eclipse SWT PI/win32/library/build.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
@rem ***************************************************************************

@rem The original build.bat source is located in /org.eclipse.swt/Eclipse SWT PI/win32/library/build.bat. It is copied during various build(s).
@rem Typically it's not ran directly, instead it is reached by build.xml's build_libraries target found in eclipse.platform.swt.binaries\bundles\org.eclipse.swt.win32.win32.x86*
@rem Typically it's not run directly, instead it is reached by build.xml's build_libraries target found in eclipse.platform.swt.binaries\bundles\org.eclipse.swt.win32.win32.*

@echo off
echo
Expand All @@ -36,7 +36,7 @@ IF "%MSVC_HOME%"=="" CALL :ECHO "'MSVC_HOME' was not provided, auto-searching fo
IF "%MSVC_HOME%"=="" CALL :FindVisualStudio "%SWT_BUILDDIR%\Microsoft\Visual Studio\$MSVC_VERSION$"
@rem Bug 574007: Path used on Azure build machines
IF "%MSVC_HOME%"=="" CALL :FindVisualStudio "%ProgramFiles(x86)%\Microsoft Visual Studio\$MSVC_VERSION$\BuildTools"
@rem Bug 578519: Common installation paths; VisualStudio is installed in x64 ProgramFiles since VS2022
@rem Bug 578519: Common installation paths; VisualStudio is installed in 64-bit ProgramFiles since VS2022
IF "%MSVC_HOME%"=="" CALL :FindVisualStudio "%ProgramFiles%\Microsoft Visual Studio\$MSVC_VERSION$\$MSVC_EDITION$"
@rem Bug 578519: Common installation paths; VisualStudio is installed in x86 ProgramFiles before VS2022
IF "%MSVC_HOME%"=="" CALL :FindVisualStudio "%ProgramFiles(x86)%\Microsoft Visual Studio\$MSVC_VERSION$\$MSVC_EDITION$"
Expand All @@ -59,25 +59,28 @@ IF "%SWT_JAVA_HOME%"=="" CALL :TryToUseJdk "%SWT_BUILDDIR%\Java\Oracle\jdk1.8.0-
@rem Note that first found JDK wins, so sort them by order of preference.
IF "%SWT_JAVA_HOME%"=="" CALL :TryToUseJdk "%ProgramFiles%\Java\jdk*"
IF "%SWT_JAVA_HOME%"=="" CALL :TryToUseJdk "%ProgramFiles%\AdoptOpenJDK\jdk*"
IF "%SWT_JAVA_HOME%"=="" CALL :TryToUseJdk "%JAVA_HOME%"
@rem Report
IF NOT EXIST "%SWT_JAVA_HOME%" (
CALL :ECHO "WARNING: x64 Java JDK not found. Please set SWT_JAVA_HOME to your JDK directory."
CALL :ECHO "WARNING: 64-bit Java JDK not found. Please set SWT_JAVA_HOME to your JDK directory."
CALL :ECHO " Refer steps for SWT Windows native setup: https://www.eclipse.org/swt/swt_win_native.php"
) ELSE (
CALL :ECHO "SWT_JAVA_HOME x64: %SWT_JAVA_HOME%"
CALL :ECHO "SWT_JAVA_HOME 64-bit: %SWT_JAVA_HOME%"
)

@rem -----------------------
IF NOT "x.%1"=="x.x86_64" (
CALL :ECHO "ERROR: 32-bit builds are no longer supported."
IF "x.%1"=="x.x86_64" (
set BUILD_ARCH=x64
IF "x.%OUTPUT_DIR%"=="x." set OUTPUT_DIR=..\..\..\org.eclipse.swt.win32.win32.x86_64
) ELSE IF "x.%1"=="x.aarch64" (
set BUILD_ARCH=arm64
IF "x.%OUTPUT_DIR%"=="x." set OUTPUT_DIR=..\..\..\org.eclipse.swt.win32.win32.aarch64
) ELSE (
CALL :ECHO "ERROR: %1 builds are no longer supported."
EXIT /B 1
)

set PROCESSOR_ARCHITECTURE=AMD64
IF "x.%OUTPUT_DIR%"=="x." set OUTPUT_DIR=..\..\..\org.eclipse.swt.win32.win32.x86_64

set CFLAGS=-DJNI64
call "%MSVC_HOME%\VC\Auxiliary\Build\vcvarsall.bat" x64
call "%MSVC_HOME%\VC\Auxiliary\Build\vcvarsall.bat" %BUILD_ARCH%
shift

@rem if call to vcvarsall.bat (which sets up environment) silently fails, then provide advice to user.
Expand Down Expand Up @@ -117,6 +120,7 @@ GOTO :EOF
CALL :FindVisualStudio3 "%~1" "%~2" "Community"
CALL :FindVisualStudio3 "%~1" "%~2" "Enterprise"
CALL :FindVisualStudio3 "%~1" "%~2" "Professional"
CALL :FindVisualStudio3 "%~1" "%~2" "Preview"
) ELSE (
CALL :FindVisualStudio3 "%~1" "%~2" "%MSVC_EDITION%"
)
Expand Down
6 changes: 3 additions & 3 deletions bundles/org.eclipse.swt/Eclipse SWT PI/win32/library/os_custom.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ BOOL Validate_AllowDarkModeForWindow(const BYTE* functionPtr)
* an ATOM value of 0xA91E which is unlikely to change
*/

#ifdef _M_X64
#if defined(_M_X64) || defined(_M_ARM64)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and other similar changes are not exactly correct, because on aarch64 disasm can't match the expected bytes. If you're not going to implement it now, it would be more correct to change this to

#elif defined(_M_ARM64)
    // Not implemented yet
    return FALSE;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the above objection at all.

What is the meaning of "aarch64 disasm can't match the expected bytes"? What is "aarch64 disasm" to do with this change in the C code?

What does it mean by "If you're not going to implement it now"? implement what exactly? The modified code compiles successfully with the Microsoft C/C++ native compiler for Arm64, so what kind of "implement" is needed here? I'm quite confused, to be honest.

If you object to the use of those predefined macros in the C code, they are specifically meant to be used with the Microsoft C/C++ compiler, targeting the Windows OS, as documented here. This C code is not for any other OS but the Windows OS, so naturally the code is specifically designed to be compiled by the dominant compiler in the Windows system (Microsoft Visual Studio Suite).

The build.bat command file was also designed for Microsoft Visual Studio compiler tools, and my changes are to continue the usage of the same tools, for generating the native binaries for the Windows on Arm64 platform.

If you object to the use of Microsoft C/C++ tools here for the Windows OS, then this would become a huge topic that this PR can't address.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and other similar changes are not exactly correct, because on aarch64 disasm can't match the expected bytes. If you're not going to implement it now, it would be more correct to change this to

#elif defined(_M_ARM64)
    // Not implemented yet
    return FALSE;

@SyntevoAlex
Looks like acronym "disasm" caused some confusion:
aarch64 "disasm" actually stands for aarch64 disassembler

@chirontt
IMHO, above comment only meant to re-arrange arm64 code something as below:
#ifdef _M_X64
{
// Existing conditions for TRUE cases, which my apply only to _M_X64 only
return TRUE;
}
return FALSE;
#elif defined(_M_ARM64)
{
// TODO: Add Conditions for any known TRUE cases for arm64
// Not implemented yet
}
return FALSE;
#else
#error Unsupported processor type
#endif

Please correct me if I got it wrong. Thanks!

Copy link
Member

@SyntevoAlex SyntevoAlex Sep 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code in question tests compiled code (machine instructions, which can be seen as assembly code, which I described as disasm for shorthand). It verifies that machine code in a Windows library matches what's expected. This is because the APIs are not documented, but we still need them, so we need to check if at least it's still the API we expected.

Surely aarch64 machine code differs from x86_64 machine code. That's the whole point of a different architecture. Otherwise you could just run x86_64 native libraries on aarch64.

Therefore, even if the code "compiles" for you, it will not work. To fix that, you have to implement checks specific to aarch64.

If you're not implementing that now, then change the code as I suggested, so that it's more clear that it's not implemented and doesn't look like "it's good".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically, as a result of NOT implementing the new checks, Dark Theme will stop working properly (scrollbars will be light, etc).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I can see what you meant now. Thanks for the explanation. And here's a screenshot of Eclipse SDK in dark mode (built myself and running on my WoA box):

image

As it shows, the scrollbars are not dark, so the code blocks within the

#if defined(_M_X64) || defined(_M_ARM64)

don't work properly in Windows Arm64; it may also not work if it just does return FALSE; instead, as suggested (I'll rebuild it and check it out later.)

In the mean time, I'll withdraw this PR, as it's become clear now that it's not useful, because the build infrastructure for building Windows on Arm64 artifacts is not yet in place to support it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aarch64 disasm taken care of at #1048

/* Win10 builds from 20236 */
if ((functionPtr[0x52] == 0xBA) && // mov edx,
(*(const DWORD*)(functionPtr + 0x53) == 0xA91E)) // 0A91Eh
Expand Down Expand Up @@ -111,7 +111,7 @@ TYPE_AllowDarkModeForWindow Locate_AllowDarkModeForWindow()

BOOL Validate_AllowDarkModeForWindowWithTelemetryId(const BYTE* functionPtr)
{
#ifdef _M_X64
#if defined(_M_X64) || defined(_M_ARM64)
/* This function is rather long, but it uses an ATOM value of 0xA91E which is unlikely to change */

/* Win10 builds from 21301 */
Expand Down Expand Up @@ -198,7 +198,7 @@ JNIEXPORT jboolean JNICALL OS_NATIVE(AllowDarkModeForWindow)

BOOL Validate_SetPreferredAppMode(const BYTE* functionPtr)
{
#ifdef _M_X64
#if defined(_M_X64) || defined(_M_ARM64)
/*
* This function is very simple, so validate entire body.
* The only thing we don't know is the variable address.
Expand Down
13 changes: 10 additions & 3 deletions bundles/org.eclipse.swt/buildSWT.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@
<param name="library_count" value="7"/>
<param name="fragment" value="org.eclipse.swt.gtk.linux.x86_64"/>
</antcall>
<antcall target="check_fragment_libraries">
<param name="library_count" value="4"/>
<param name="fragment" value="org.eclipse.swt.win32.win32.aarch64"/>
</antcall>
<antcall target="check_fragment_libraries">
<param name="library_count" value="4"/>
<param name="fragment" value="org.eclipse.swt.win32.win32.x86_64"/>
Expand All @@ -86,15 +90,15 @@
<target name="check_fragment_libraries" depends="get_version">
<echo>Checking ${fragment}</echo>
<property name="checkdir" value="~/build/check_libraries"/>
<property name="library_count" value="31"/>
<property name="library_count" value="35"/>
<property name="fragment" value=""/>
<fileset id="match" dir="${repo.bin}/bundles/${fragment}" includes="**/org.eclipse.swt.gtk.linux.aarch64/**, **/org.eclipse.swt.gtk.linux.ppc64le/**, **/org.eclipse.swt.gtk.linux.x86_64/**, **/org.eclipse.swt.win32.win32.x86_64/**, **/org.eclipse.swt.cocoa.macosx.aarch64/**, **/org.eclipse.swt.cocoa.macosx.x86_64/**">
<fileset id="match" dir="${repo.bin}/bundles/${fragment}" includes="**/org.eclipse.swt.gtk.linux.aarch64/**, **/org.eclipse.swt.gtk.linux.ppc64le/**, **/org.eclipse.swt.gtk.linux.x86_64/**, **/org.eclipse.swt.win32.win32.aarch64/**, **/org.eclipse.swt.win32.win32.x86_64/**, **/org.eclipse.swt.cocoa.macosx.aarch64/**, **/org.eclipse.swt.cocoa.macosx.x86_64/**">
<filename regex="[0-9][0-9][0-9][0-9]"/>
<filename regex="${swt_version}"/>
<exclude name="**/.git/**"/>
</fileset>
<echo>Matched files ${toString:match}</echo>
<fileset id="not_match" dir="${repo.bin}/bundles/${fragment}" includes="**/org.eclipse.swt.gtk.linux.aarch64/**, **/org.eclipse.swt.gtk.linux.ppc64le/**, **/org.eclipse.swt.gtk.linux.x86_64/**, **/org.eclipse.swt.win32.win32.x86_64/**, **/org.eclipse.swt.cocoa.macosx.aarch64/**, **/org.eclipse.swt.cocoa.macosx.x86_64/**">
<fileset id="not_match" dir="${repo.bin}/bundles/${fragment}" includes="**/org.eclipse.swt.gtk.linux.aarch64/**, **/org.eclipse.swt.gtk.linux.ppc64le/**, **/org.eclipse.swt.gtk.linux.x86_64/**, **/org.eclipse.swt.win32.win32.aarch64/**, **/org.eclipse.swt.win32.win32.x86_64/**, **/org.eclipse.swt.cocoa.macosx.aarch64/**, **/org.eclipse.swt.cocoa.macosx.x86_64/**">
<filename regex="[0-9][0-9][0-9][0-9]"/>
<filename regex="${swt_version}" negate="true"/>
<exclude name="**/.git/**"/>
Expand Down Expand Up @@ -255,6 +259,9 @@
<antcall target="build_classes">
<param name="cp" value=".classpath_win32"/>
</antcall>
<antcall target="build_classes">
<param name="cp" value=".classpath_win32_aarch64"/>
</antcall>

<antcall target="check_preprocessing"/>
</target>
Expand Down
5 changes: 5 additions & 0 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,11 @@
<version>${tycho.version}</version>
<configuration>
<environments>
<environment>
<os>win32</os>
<ws>win32</ws>
<arch>aarch64</arch>
</environment>
<environment>
<os>win32</os>
<ws>win32</ws>
Expand Down