Skip to content

Fix GH-16839: Error on building Opcache JIT for Windows ARM64 #16841

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

Closed
wants to merge 1 commit into from
Closed

Fix GH-16839: Error on building Opcache JIT for Windows ARM64 #16841

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions ext/opcache/config.w32
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ ARG_ENABLE("opcache", "whether to enable Zend OPcache support", "yes");

if (PHP_OPCACHE != "no") {

ARG_ENABLE("opcache-jit", "whether to enable JIT", "yes");
ARG_ENABLE("opcache-jit", "whether to enable JIT (not supported for ARM64)", "yes");

ZEND_EXTENSION('opcache', "\
ZendAccelerator.c \
Expand All @@ -23,7 +23,9 @@ if (PHP_OPCACHE != "no") {
ADD_EXTENSION_DEP('opcache', 'pcre');

if (PHP_OPCACHE_JIT == "yes") {
if (CHECK_HEADER_ADD_INCLUDE("ir/ir.h", "CFLAGS_OPCACHE", PHP_OPCACHE + ";ext\\opcache\\jit")) {
if (TARGET_ARCH == 'arm64') {
WARNING("JIT not enabled; not yet supported for ARM64");
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on https://github.com/dstogov/ir/tree/master/dynasm I was under the impression ARM is supported, so it isn't or only ARM on Windows?

Copy link
Member Author

Choose a reason for hiding this comment

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

Supported is only aarch64 on POSIX systems. See #16839 (comment) f.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps it might be wise to specify in the warning message that JIT is not support on ARM64 on Windows (indicating that it is supported on POSIX systems)

Copy link
Member Author

Choose a reason for hiding this comment

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

This file is part of the Windows build chain; it is not used for building on POSIX systems.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well yeah, but the warning is meant for humans

Copy link
Member Author

Choose a reason for hiding this comment

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

$ configure --with-toolset=gcc
PHP Version: 8.5.0-dev

Saving configure options to config.nice.bat
ERROR: Unsupported toolset 'gcc'

Copy link
Contributor

Choose a reason for hiding this comment

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

My point was that the warning could be misinterpreted by windows-only developers who don't know much about JIT, leding them to think that JIT is not supported at all on arm64, when they could be instead directed to use WSL, where JIT is available, but ah well :)

} else if (CHECK_HEADER_ADD_INCLUDE("ir/ir.h", "CFLAGS_OPCACHE", PHP_OPCACHE + ";ext\\opcache\\jit")) {
var dasm_flags = (X64 ? "-D X64=1" : "") + (X64 ? " -D X64WIN=1" : "") + " -D WIN=1";
var ir_target = (X64 ? "IR_TARGET_X64" : "IR_TARGET_X86");
var ir_src = "ir_strtab.c ir_cfg.c ir_sccp.c ir_gcm.c ir_ra.c ir_save.c \
Expand Down