Use ASM unknown function trampolines on GN #1530
Conversation
|
Author jbauman42 not on autobuild list. Waiting for curator authorization before starting CI build. |
1 similar comment
|
Author jbauman42 not on autobuild list. Waiting for curator authorization before starting CI build. |
|
CI Vulkan-Loader build queued with queue ID 232655. |
|
CI Vulkan-Loader build # 2655 running. |
|
CI Vulkan-Loader build # 2655 failed. |
loader/unknown_ext_chain_gas_aarch.S
Outdated
|
|
||
| .macro PhysDevExtTramp num | ||
| .global vkPhysDevExtTramp\num | ||
| .type vkPhysDevExtTramp\num, @function |
There was a problem hiding this comment.
This change is causing the MacOS CI to fail - seems that github actions CI is running on x86_64 so isn't affected.
I can send you the full log, but the main error is this - related to the .type vkPhysDevExtTramp\num, @function
<instantiation>:2:1: error: unknown directive
.type vkPhysDevExtTramp0, @function
^
/Users/lunarg/.jenkins/w3/Release64-3/Vulkan-Loader/loader/unknown_ext_chain_gas_aarch.S:208:5: note: while in macro instantiation
PhysDevExtTramp 0
^
There was a problem hiding this comment.
loading initial cache file /Users/lunarg/.jenkins/w3/Release64-3/Vulkan-Loader/external/helper.cmake
-- The C compiler identification is AppleClang 15.0.0.15000040
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Found Git: /usr/bin/git (found version "2.39.3 (Apple Git-145)")
-- Looking for alloca.h
-- Looking for alloca.h - found
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- The ASM compiler identification is Clang with GNU-like command-line
-- Found assembler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc
-- Found PkgConfig: /opt/homebrew/bin/pkg-config (found version "0.29.2")
-- The CXX compiler identification is AppleClang 15.0.0.15000040
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Python3: /opt/homebrew/Frameworks/Python.framework/Versions/3.10/bin/python3.10 (found version "3.10.8") found components: Interpreter
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/lunarg/.jenkins/w3/Release64-3/Vulkan-Loader/build
The CMake configure indicates what version of the compiler is being used, as well as the assembler used.
-- The C compiler identification is AppleClang 15.0.0.15000040
-- The ASM compiler identification is Clang with GNU-like command-line
So apparently the .type directive isn't supported with apple clang? Or its malformed? I'll look into it and see if there is a 'quick fix'.
There was a problem hiding this comment.
Oh, I didn't realize it wouldn't work on Mac. We could probably move it inside #if defined(__ELF__) below; it's not critical to this patch.
There was a problem hiding this comment.
I'm happy to implement the move, just not wanting to step on your toes if you are going to move the .type into the #if defined(ELF) blocks.
The GN build doesn't currently support unknown function trampolines. To fix that, we can use the assembly trampolines. To generate the asm_offset assembly file, we can build it in a static library and pass -S to it; that causes the assembly file to be copied into the .a file. The python script can then extract it. This requires access to the "ar" executable, so this mechanism is only used if a path to ar is specified in //build_overrides/vulkan_loader.gni. To fix incremental builds, "gen_defines.asm" must be included using "#include", not ".include", since .include isn't a preprocessor directive and isn't picked up by clang's depfile creation.
|
Author jbauman42 not on autobuild list. Waiting for curator authorization before starting CI build. |
1 similar comment
|
Author jbauman42 not on autobuild list. Waiting for curator authorization before starting CI build. |
|
CI Vulkan-Loader build queued with queue ID 234079. |
|
CI Vulkan-Loader build # 2658 running. |
|
CI Vulkan-Loader build # 2658 passed. |
|
Thanks for fixing this hole in the gn build! |
There was a problem hiding this comment.
This fails to roll into Dawn, Chromium and probably ANGLE, e.g. https://chromium-review.googlesource.com/c/chromium/src/+/5784260.
Please fix.
| library_type = "static_library" | ||
| } | ||
| support_unknown_function_handling = false | ||
| if (ar_path != "" && !is_win && (current_cpu == "arm64" || current_cpu == "x86_64")) { |
There was a problem hiding this comment.
Should be: if (defined(ar_path) ...
Otherwise GN gen fails:
https://dawn-review.googlesource.com/c/dawn/+/202314
https://ci.chromium.org/ui/p/dawn/builders/try/mac-rel/b8739735338416185889/overview
https://logs.chromium.org/logs/dawn/buildbucket/cr-buildbucket/8739735338416185889/+/u/gn_gen/stdout
ERROR at //third_party/vulkan-loader/src/BUILD.gn:101:7: Undefined identifier
if (ar_path != "" && !is_win && (current_cpu == "arm64" || current_cpu == "x86_64")) {
@zoddicus
There was a problem hiding this comment.
I have merged a PR with the defined(ar_path) fix.
Let me know if that fixes it or what further changes are required.
There was a problem hiding this comment.
Thank you, Charles! The fix looks good here https://chromium-review.googlesource.com/c/angle/angle/+/5785295.
The GN build doesn't currently support unknown function trampolines. To fix that, we can use the assembly trampolines. To generate the asm_offset assembly file, we can build it in a static library and pass -S to it; that causes the assembly file to be copied into the .a file. The python script can then extract it. This requires access to the "ar" executable, so this mechanism is only used if a path to ar is specified in //build_overrides/vulkan_loader.gni.
To fix incremental builds, "gen_defines.asm" must be included using "#include", not ".include", since .include isn't a preprocessor directive and isn't picked up by clang's depfile creation.