Skip to content

RuntimeLibcalls: Remove darwin override of half convert libcalls #148782

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

Conversation

arsenm
Copy link
Contributor

@arsenm arsenm commented Jul 15, 2025

These are already the default calls set for these conversions, so
they should not require explicit setting. The non-default cases are
currently overridden in ARMISelLowering. Just delete this until
the list of calls and lowering decisions are separated.

This was added back in 6402ad2. It
appears to not be relevant for AArch64, where calls appear to never
be used for these. It also appears to not be relevant for x86, where
the default calls seem to always end up used anyway.

@arsenm arsenm requested a review from ahmedbougacha July 15, 2025 06:19
@arsenm arsenm marked this pull request as ready for review July 15, 2025 06:20
@llvmbot
Copy link
Member

llvmbot commented Jul 15, 2025

@llvm/pr-subscribers-backend-arm

@llvm/pr-subscribers-llvm-ir

Author: Matt Arsenault (arsenm)

Changes

These are already the default calls set for these conversions, so
they should not require explicit setting. The non-default cases are
currently overridden in ARMISelLowering. Just delete this until
the list of calls and lowering decisions are separated.

This was added back in 6402ad2. It
appears to not be relevant for AArch64, where calls appear to never
be used for these. It also appears to not be relevant for x86, where
the default calls seem to always end up used anyway.


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

1 Files Affected:

  • (modified) llvm/lib/IR/RuntimeLibcalls.cpp (-9)
diff --git a/llvm/lib/IR/RuntimeLibcalls.cpp b/llvm/lib/IR/RuntimeLibcalls.cpp
index df529bc013cac..8ca31dff900cb 100644
--- a/llvm/lib/IR/RuntimeLibcalls.cpp
+++ b/llvm/lib/IR/RuntimeLibcalls.cpp
@@ -77,15 +77,6 @@ void RuntimeLibcallsInfo::initLibcalls(const Triple &TT,
       setLibcallImpl(RTLIB::UNWIND_RESUME, RTLIB::_Unwind_SjLj_Resume);
   }
 
-  // A few names are different on particular architectures or environments.
-  if (TT.isOSDarwin()) {
-    // For f16/f32 conversions, Darwin uses the standard naming scheme,
-    // instead of the gnueabi-style __gnu_*_ieee.
-    // FIXME: What about other targets?
-    setLibcallImpl(RTLIB::FPEXT_F16_F32, RTLIB::__extendhfsf2);
-    setLibcallImpl(RTLIB::FPROUND_F32_F16, RTLIB::__truncsfhf2);
-  }
-
   if (TT.isOSOpenBSD()) {
     setLibcallImpl(RTLIB::STACKPROTECTOR_CHECK_FAIL, RTLIB::Unsupported);
   }

@llvmbot
Copy link
Member

llvmbot commented Jul 15, 2025

@llvm/pr-subscribers-tablegen

Author: Matt Arsenault (arsenm)

Changes

These are already the default calls set for these conversions, so
they should not require explicit setting. The non-default cases are
currently overridden in ARMISelLowering. Just delete this until
the list of calls and lowering decisions are separated.

This was added back in 6402ad2. It
appears to not be relevant for AArch64, where calls appear to never
be used for these. It also appears to not be relevant for x86, where
the default calls seem to always end up used anyway.


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

1 Files Affected:

  • (modified) llvm/lib/IR/RuntimeLibcalls.cpp (-9)
diff --git a/llvm/lib/IR/RuntimeLibcalls.cpp b/llvm/lib/IR/RuntimeLibcalls.cpp
index df529bc013cac..8ca31dff900cb 100644
--- a/llvm/lib/IR/RuntimeLibcalls.cpp
+++ b/llvm/lib/IR/RuntimeLibcalls.cpp
@@ -77,15 +77,6 @@ void RuntimeLibcallsInfo::initLibcalls(const Triple &TT,
       setLibcallImpl(RTLIB::UNWIND_RESUME, RTLIB::_Unwind_SjLj_Resume);
   }
 
-  // A few names are different on particular architectures or environments.
-  if (TT.isOSDarwin()) {
-    // For f16/f32 conversions, Darwin uses the standard naming scheme,
-    // instead of the gnueabi-style __gnu_*_ieee.
-    // FIXME: What about other targets?
-    setLibcallImpl(RTLIB::FPEXT_F16_F32, RTLIB::__extendhfsf2);
-    setLibcallImpl(RTLIB::FPROUND_F32_F16, RTLIB::__truncsfhf2);
-  }
-
   if (TT.isOSOpenBSD()) {
     setLibcallImpl(RTLIB::STACKPROTECTOR_CHECK_FAIL, RTLIB::Unsupported);
   }

@llvmbot
Copy link
Member

llvmbot commented Jul 15, 2025

@llvm/pr-subscribers-backend-x86

Author: Matt Arsenault (arsenm)

Changes

These are already the default calls set for these conversions, so
they should not require explicit setting. The non-default cases are
currently overridden in ARMISelLowering. Just delete this until
the list of calls and lowering decisions are separated.

This was added back in 6402ad2. It
appears to not be relevant for AArch64, where calls appear to never
be used for these. It also appears to not be relevant for x86, where
the default calls seem to always end up used anyway.


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

1 Files Affected:

  • (modified) llvm/lib/IR/RuntimeLibcalls.cpp (-9)
diff --git a/llvm/lib/IR/RuntimeLibcalls.cpp b/llvm/lib/IR/RuntimeLibcalls.cpp
index df529bc013cac..8ca31dff900cb 100644
--- a/llvm/lib/IR/RuntimeLibcalls.cpp
+++ b/llvm/lib/IR/RuntimeLibcalls.cpp
@@ -77,15 +77,6 @@ void RuntimeLibcallsInfo::initLibcalls(const Triple &TT,
       setLibcallImpl(RTLIB::UNWIND_RESUME, RTLIB::_Unwind_SjLj_Resume);
   }
 
-  // A few names are different on particular architectures or environments.
-  if (TT.isOSDarwin()) {
-    // For f16/f32 conversions, Darwin uses the standard naming scheme,
-    // instead of the gnueabi-style __gnu_*_ieee.
-    // FIXME: What about other targets?
-    setLibcallImpl(RTLIB::FPEXT_F16_F32, RTLIB::__extendhfsf2);
-    setLibcallImpl(RTLIB::FPROUND_F32_F16, RTLIB::__truncsfhf2);
-  }
-
   if (TT.isOSOpenBSD()) {
     setLibcallImpl(RTLIB::STACKPROTECTOR_CHECK_FAIL, RTLIB::Unsupported);
   }

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

LGTM, this matches what I've seen while inspecting the tablegen output

@arsenm arsenm force-pushed the users/arsenm/runtime-libcalls/use-tablegen-default-handling branch from ef419d6 to f5462a6 Compare July 15, 2025 07:20
@arsenm arsenm force-pushed the users/arsenm/runtime-libcalls/remove-darwin-half-convert-libcall-case branch from 39867e3 to b5bbf7f Compare July 15, 2025 07:21
@arsenm arsenm force-pushed the users/arsenm/runtime-libcalls/use-tablegen-default-handling branch from f5462a6 to 5843fba Compare July 28, 2025 02:43
@arsenm arsenm force-pushed the users/arsenm/runtime-libcalls/remove-darwin-half-convert-libcall-case branch from b5bbf7f to 0b21d22 Compare July 28, 2025 02:43
@arsenm arsenm force-pushed the users/arsenm/runtime-libcalls/use-tablegen-default-handling branch from 5843fba to a8d3505 Compare August 3, 2025 00:41
@arsenm arsenm force-pushed the users/arsenm/runtime-libcalls/remove-darwin-half-convert-libcall-case branch from 0b21d22 to 9404c93 Compare August 3, 2025 00:41
@arsenm arsenm force-pushed the users/arsenm/runtime-libcalls/remove-darwin-half-convert-libcall-case branch from 9404c93 to 20a6035 Compare August 3, 2025 16:32
@arsenm arsenm force-pushed the users/arsenm/runtime-libcalls/use-tablegen-default-handling branch from a8d3505 to 744161e Compare August 3, 2025 16:32
Base automatically changed from users/arsenm/runtime-libcalls/use-tablegen-default-handling to main August 3, 2025 23:32
@arsenm arsenm force-pushed the users/arsenm/runtime-libcalls/remove-darwin-half-convert-libcall-case branch 2 times, most recently from b040c7a to d564efc Compare August 4, 2025 00:42
Copy link
Contributor Author

arsenm commented Aug 4, 2025

Merge activity

  • Aug 4, 12:43 AM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 4, 12:51 AM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 4, 1:21 AM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 4, 1:30 AM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 4, 1:33 AM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 4, 1:50 AM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 4, 1:56 AM UTC: Graphite rebased this pull request as part of a merge.
  • Aug 4, 2:01 AM UTC: Graphite rebased this pull request as part of a merge.

@arsenm arsenm force-pushed the users/arsenm/runtime-libcalls/remove-darwin-half-convert-libcall-case branch 6 times, most recently from 1545d0e to bd93ef9 Compare August 4, 2025 01:55
These are already the default calls set for these conversions, so
they should not require explicit setting. The non-default cases are
currently overridden in ARMISelLowering. Just delete this until
the list of calls and lowering decisions are separated.

This was added back in 6402ad2. It
appears to not be relevant for AArch64, where calls appear to never
be used for these. It also appears to not be relevant for x86, where
the default calls seem to always end up used anyway.
@arsenm arsenm force-pushed the users/arsenm/runtime-libcalls/remove-darwin-half-convert-libcall-case branch from bd93ef9 to 0c1bb90 Compare August 4, 2025 02:00
@arsenm arsenm merged commit 5b528a1 into main Aug 4, 2025
7 of 9 checks passed
@arsenm arsenm deleted the users/arsenm/runtime-libcalls/remove-darwin-half-convert-libcall-case branch August 4, 2025 02:06
@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 4, 2025

LLVM Buildbot has detected a new failure on builder lldb-x86_64-debian running on lldb-x86_64-debian while building llvm at step 6 "test".

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

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-unit :: Host/./HostTests/75/125 (259 of 3128)
PASS: lldb-api :: functionalities/lazy-loading/TestLazyLoading.py (260 of 3128)
PASS: lldb-api :: functionalities/tail_call_frames/thread_step_out_message/TestArtificialFrameStepOutMessage.py (261 of 3128)
PASS: lldb-api :: types/TestIntegerType.py (262 of 3128)
PASS: lldb-api :: functionalities/tail_call_frames/disambiguate_tail_call_seq/TestDisambiguateTailCallSeq.py (263 of 3128)
PASS: lldb-api :: functionalities/exec/TestExec.py (264 of 3128)
PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx-simulators/unique_ptr/TestDataFormatterLibcxxUniquePtrSimulator.py (265 of 3128)
PASS: lldb-api :: python_api/type/TestTypeList.py (266 of 3128)
PASS: lldb-api :: functionalities/thread/concurrent_events/TestConcurrentManyBreakpoints.py (267 of 3128)
PASS: lldb-api :: functionalities/tail_call_frames/disambiguate_paths_to_common_sink/TestDisambiguatePathsToCommonSink.py (268 of 3128)
FAIL: lldb-api :: tools/lldb-dap/output/TestDAP_output.py (269 of 3128)
******************** TEST 'lldb-api :: tools/lldb-dap/output/TestDAP_output.py' FAILED ********************
Script:
--
/usr/bin/python3 /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./lib --env LLVM_INCLUDE_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/include --env LLVM_TOOLS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./bin --arch x86_64 --build-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex --lldb-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/lldb --compiler /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/clang --dsymutil /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./bin --lldb-obj-root /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb --lldb-libs-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./lib --cmake-build-type Release -t /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/output -p TestDAP_output.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 22.0.0git (https://github.com/llvm/llvm-project.git revision 5b528a1041fb6b64e5722f1bc600eb0617118a3e)
  clang revision 5b528a1041fb6b64e5722f1bc600eb0617118a3e
  llvm revision 5b528a1041fb6b64e5722f1bc600eb0617118a3e
Skipping the following test categories: ['libc++', 'msvcstl', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
Change dir to: /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/tools/lldb-dap/output
runCmd: settings clear --all

output: 

runCmd: settings set symbols.enable-external-lookup false

output: 

runCmd: settings set target.inherit-tcc true

output: 

runCmd: settings set target.disable-aslr false

output: 

runCmd: settings set target.detach-on-error false

output: 


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.

5 participants