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.

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

PR for llvm/llvm-project#81084 #81085

Merged
merged 1 commit into from
Feb 9, 2024
Merged

Conversation

llvmbot
Copy link
Collaborator

@llvmbot llvmbot commented Feb 8, 2024

resolves #81084

@llvmbot llvmbot requested a review from a team as a code owner February 8, 2024 04:09
@llvmbot llvmbot added this to the LLVM 18.X Release milestone Feb 8, 2024
@llvmbot
Copy link
Collaborator Author

llvmbot commented Feb 8, 2024

@MaskRay What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Collaborator Author

llvmbot commented Feb 8, 2024

@llvm/pr-subscribers-libunwind

Author: None (llvmbot)

Changes

resolves llvm/llvm-project#81084


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

1 Files Affected:

  • (modified) libunwind/CMakeLists.txt (+19)
diff --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index bb1b052f61d875..806d5a783ec39c 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -21,6 +21,7 @@ set(LIBUNWIND_LIBCXX_PATH "${CMAKE_CURRENT_LIST_DIR}/../libcxx" CACHE PATH
         "Specify path to libc++ source.")
 
 include(GNUInstallDirs)
+include(CheckSymbolExists)
 
 #===============================================================================
 # Setup CMake Options
@@ -96,6 +97,20 @@ endif()
 option(LIBUNWIND_HIDE_SYMBOLS
   "Do not export any symbols from the static library." ${LIBUNWIND_DEFAULT_HIDE_SYMBOLS})
 
+# If toolchain is FPXX, we switch to FP64 to save the full FPRs. See:
+# https://web.archive.org/web/20180828210612/https://dmz-portal.mips.com/wiki/MIPS_O32_ABI_-_FR0_and_FR1_Interlinking
+check_symbol_exists(__mips_hard_float "" __MIPSHF)
+check_symbol_exists(_ABIO32 "" __MIPS_O32)
+if (__MIPSHF AND __MIPS_O32)
+  file(WRITE ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/mips_is_fpxx.c
+    "#if __mips_fpr != 0\n"
+    "# error\n"
+    "#endif\n")
+  try_compile(MIPS_FPABI_FPXX ${CMAKE_BINARY_DIR}
+    ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/CMakeTmp/mips_is_fpxx.c
+    CMAKE_FLAGS -DCMAKE_C_LINK_EXECUTABLE='echo')
+endif()
+
 #===============================================================================
 # Configure System
 #===============================================================================
@@ -179,6 +194,10 @@ if (WIN32)
   add_compile_flags_if_supported(-Wno-dll-attribute-on-redeclaration)
 endif()
 
+if (MIPS_FPABI_FPXX)
+  add_compile_flags(-mfp64)
+endif()
+
 # Get feature flags.
 # Exceptions
 # Catches C++ exceptions only and tells the compiler to assume that extern C

Libunwind supports FP64 and FP32 modes, but not FPXX. The reason is
that, FP64 and FP32 have different way to save/restore FPRs. If
libunwind is built as FPXX, we have no idea which one should we use.

It's not due to the code bug, but rather the nature of FPXX.
FPXX is an ABI which uses only a common subset of FR=1(FP64) and FR=0
(FP32).
So that FPXX binaries can link with both FP64 and FP32 ones, aka.
    FPXX + FP32 -> FP32
    FPXX + FP64 -> FP64

While for libunwind, we should save/restore all of FPRs. If we use FPXX,
we can only save/restore a common subset of FPRs, instead of superset.

If libunwind is built as FP64, it will interoperatable with FPXX/FP64
APPs, and if it is built as FP32, it will interoperatable with
FP32/FPXX. Currently most of O32 APPs are FPXX or FP64, while few are
FP32.

So if the compiler is FPXX, which is the default value of most
toolchain, let's switch it to FP64.

Co-authored-by: YunQiang Su <yunqiang.su@cipunited.com>
(cherry picked from commit 4520b47)
@tstellar tstellar merged commit 2b033a3 into llvm:release/18.x Feb 9, 2024
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants