Skip to content

[LLD][COFF] Add support for -includeoptional on ARM64X #126300

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.

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

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

cjacek
Copy link
Contributor

@cjacek cjacek commented Feb 7, 2025

Include symbols from both symbol tables.

Include symbols from both symbol tables.
@cjacek
Copy link
Contributor Author

cjacek commented Feb 7, 2025

Since this is an LLD extension, we can define its behavior on ARM64X. Unlike -include, I applied it to both symbol tables. Because it isn't allowed in .drectve, there would be no way to apply it to both tables otherwise. The optional nature of the argument should make it harmless in cases where the intent is to apply it to only one table.

@llvmbot
Copy link
Member

llvmbot commented Feb 7, 2025

@llvm/pr-subscribers-platform-windows
@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-coff

Author: Jacek Caban (cjacek)

Changes

Include symbols from both symbol tables.


Full diff: https://github.com/llvm/llvm-project/pull/126300.diff

2 Files Affected:

  • (modified) lld/COFF/Driver.cpp (+7-7)
  • (added) lld/test/COFF/arm64x-includeoptional.s (+17)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 281510b7ac6ea6..7ffd3f2219757c 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -2557,14 +2557,14 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
         // it.
         if (symtab.findUnderscore("_load_config_used"))
           symtab.addGCRoot(symtab.mangle("_load_config_used"));
-      });
 
-      if (args.hasArg(OPT_include_optional)) {
-        // Handle /includeoptional
-        for (auto *arg : args.filtered(OPT_include_optional))
-          if (isa_and_nonnull<LazyArchive>(ctx.symtab.find(arg->getValue())))
-            ctx.symtab.addGCRoot(arg->getValue());
-      }
+        if (args.hasArg(OPT_include_optional)) {
+          // Handle /includeoptional
+          for (auto *arg : args.filtered(OPT_include_optional))
+            if (isa_and_nonnull<LazyArchive>(symtab.find(arg->getValue())))
+              symtab.addGCRoot(arg->getValue());
+        }
+      });
     } while (run());
   }
 
diff --git a/lld/test/COFF/arm64x-includeoptional.s b/lld/test/COFF/arm64x-includeoptional.s
new file mode 100644
index 00000000000000..40e885bd1c4536
--- /dev/null
+++ b/lld/test/COFF/arm64x-includeoptional.s
@@ -0,0 +1,17 @@
+// REQUIRE: aarch64
+
+// RUN: llvm-mc -filetype=obj -triple=aarch64-windows %s -o %t.arm64.obj
+// RUN: llvm-mc -filetype=obj -triple=arm64ec-windows %s -o %t.arm64ec.obj
+// RUN: llvm-mc -filetype=obj -triple=arm64ec-windows %S/Inputs/loadconfig-arm64ec.s -o %t-loadconfig-arm64ec.obj
+// RUN: llvm-mc -filetype=obj -triple=aarch64-windows %S/Inputs/loadconfig-arm64.s -o %t-loadconfig-arm64.obj
+
+// RUN: llvm-lib -machine:arm64x -out:%t-test.lib %t.arm64.obj %t.arm64ec.obj %t-loadconfig-arm64ec.obj %t-loadconfig-arm64.obj
+// RUN: lld-link -machine:arm64x -dll -noentry -out:%t.dll %t-test.lib -includeoptional:sym
+
+// RUN: llvm-readobj --hex-dump=.test %t.dll | FileCheck %s
+// CHECK: 0x180004000 01000000 01000000
+
+        .globl sym
+        .section .test,"dr"
+sym:
+        .word 1

Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

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

LGTM

As you say this option isn't a standard link.exe option, and I don't think there's much direct use of it; the main use is probably via the mingw driver via the -u option. So the main question is what behaviour makes most sense for that flag. In the end, yes, I agree that it probably makes most sense to try to pull in the symbol in both namespaces.

@cjacek cjacek merged commit 6536579 into llvm:main Feb 10, 2025
12 checks passed
@cjacek cjacek deleted the arm64x-includeoptional branch February 10, 2025 21:01
@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 10, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-sie-ubuntu-fast running on sie-linux-worker while building lld at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/17798

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'lld :: COFF/arm64x-includeoptional.s' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 3: /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/bin/llvm-mc -filetype=obj -triple=aarch64-windows /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/lld/test/COFF/arm64x-includeoptional.s -o /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/lld/test/COFF/Output/arm64x-includeoptional.s.tmp.arm64.obj
+ /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/bin/llvm-mc -filetype=obj -triple=aarch64-windows /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/llvm-project/lld/test/COFF/arm64x-includeoptional.s -o /home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/tools/lld/test/COFF/Output/arm64x-includeoptional.s.tmp.arm64.obj
/home/buildbot/buildbot-root/llvm-clang-x86_64-sie-ubuntu-fast/build/bin/llvm-mc: error: unable to get target for 'aarch64-unknown-windows-msvc', see --version and --triple.
--

********************


@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 10, 2025

LLVM Buildbot has detected a new failure on builder openmp-offload-amdgpu-runtime running on omp-vega20-0 while building lld at step 7 "Add check check-offload".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/15533

Here is the relevant piece of the build log for the reference
Step 7 (Add check check-offload) failure: test (failure)
******************** TEST 'libomptarget :: amdgcn-amd-amdhsa :: api/omp_host_call.c' FAILED ********************
Exit Code: 2

Command Output (stdout):
--
# RUN: at line 1
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp    -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src  -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib  -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/api/omp_host_call.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/api/Output/omp_host_call.c.tmp /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a && /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/api/Output/omp_host_call.c.tmp | /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/api/omp_host_call.c
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/clang -fopenmp -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test -I /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -L /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -nogpulib -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/openmp/runtime/src -Wl,-rpath,/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib -fopenmp-targets=amdgcn-amd-amdhsa /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/api/omp_host_call.c -o /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/api/Output/omp_host_call.c.tmp /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./lib/libomptarget.devicertl.a
# note: command had no output on stdout or stderr
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/runtimes/runtimes-bins/offload/test/amdgcn-amd-amdhsa/api/Output/omp_host_call.c.tmp
# note: command had no output on stdout or stderr
# error: command failed with exit status: -11
# executed command: /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/api/omp_host_call.c
# .---command stderr------------
# | FileCheck error: '<stdin>' is empty.
# | FileCheck command line:  /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/./bin/FileCheck /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/offload/test/api/omp_host_call.c
# `-----------------------------
# error: command failed with exit status: 2

--

********************


@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 10, 2025

LLVM Buildbot has detected a new failure on builder openmp-offload-sles-build-only running on rocm-worker-hw-04-sles while building lld at step 9 "Add check check-lld".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/16667

Here is the relevant piece of the build log for the reference
Step 9 (Add check check-lld) failure: test (failure)
******************** TEST 'lld :: COFF/arm64x-includeoptional.s' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 3: /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.build/bin/llvm-mc -filetype=obj -triple=aarch64-windows /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/lld/test/COFF/arm64x-includeoptional.s -o /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.build/tools/lld/test/COFF/Output/arm64x-includeoptional.s.tmp.arm64.obj
+ /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.build/bin/llvm-mc -filetype=obj -triple=aarch64-windows /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.src/lld/test/COFF/arm64x-includeoptional.s -o /home/botworker/bbot/builds/openmp-offload-sles-build/llvm.build/tools/lld/test/COFF/Output/arm64x-includeoptional.s.tmp.arm64.obj
/home/botworker/bbot/builds/openmp-offload-sles-build/llvm.build/bin/llvm-mc: error: unable to get target for 'aarch64-unknown-windows-msvc', see --version and --triple.
--

********************


cjacek added a commit that referenced this pull request Feb 10, 2025
@cjacek
Copy link
Contributor Author

cjacek commented Feb 10, 2025

I used REQUIRE instead of REQUIRES in the test. I pushed a fixup.

@llvm-ci
Copy link
Collaborator

llvm-ci commented Feb 10, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-sie-win running on sie-win-worker while building lld at step 7 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/46/builds/11876

Here is the relevant piece of the build log for the reference
Step 7 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'lld :: COFF/arm64x-includeoptional.s' FAILED ********************
Exit Code: 1

Command Output (stdout):
--
# RUN: at line 3
z:\b\llvm-clang-x86_64-sie-win\build\bin\llvm-mc.exe -filetype=obj -triple=aarch64-windows Z:\b\llvm-clang-x86_64-sie-win\llvm-project\lld\test\COFF\arm64x-includeoptional.s -o Z:\b\llvm-clang-x86_64-sie-win\build\tools\lld\test\COFF\Output\arm64x-includeoptional.s.tmp.arm64.obj
# executed command: 'z:\b\llvm-clang-x86_64-sie-win\build\bin\llvm-mc.exe' -filetype=obj -triple=aarch64-windows 'Z:\b\llvm-clang-x86_64-sie-win\llvm-project\lld\test\COFF\arm64x-includeoptional.s' -o 'Z:\b\llvm-clang-x86_64-sie-win\build\tools\lld\test\COFF\Output\arm64x-includeoptional.s.tmp.arm64.obj'
# .---command stderr------------
# | z:\b\llvm-clang-x86_64-sie-win\build\bin\llvm-mc.exe: error: unable to get target for 'aarch64-unknown-windows-msvc', see --version and --triple.
# `-----------------------------
# error: command failed with exit status: 1

--

********************


Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Feb 14, 2025
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Feb 14, 2025
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Feb 24, 2025
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Feb 24, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Mar 8, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Mar 8, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Mar 20, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Mar 20, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Apr 2, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Apr 2, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Apr 17, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Apr 17, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Apr 30, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Apr 30, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request May 15, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request May 15, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request May 29, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request May 29, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Jun 13, 2025
SquallATF pushed a commit to SquallATF/llvm-project that referenced this pull request Jun 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants