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

[Internalize] Preserve built-in functions #69216

Closed
wants to merge 3 commits into from

Conversation

DianQK
Copy link
Member

@DianQK DianQK commented Oct 16, 2023

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 16, 2023

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-llvm-ir

Author: DianQK (DianQK)

Changes

Fixes #65965.

Related:


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

3 Files Affected:

  • (modified) llvm/include/llvm/IR/RuntimeLibcalls.def (+83-6)
  • (modified) llvm/lib/Transforms/IPO/Internalize.cpp (+10)
  • (added) llvm/test/Transforms/Internalize/built-in.ll (+18)
diff --git a/llvm/include/llvm/IR/RuntimeLibcalls.def b/llvm/include/llvm/IR/RuntimeLibcalls.def
index 6ec98e278988428..3e09b998e6135d7 100644
--- a/llvm/include/llvm/IR/RuntimeLibcalls.def
+++ b/llvm/include/llvm/IR/RuntimeLibcalls.def
@@ -72,19 +72,53 @@ HANDLE_LIBCALL(UREM_I64, "__umoddi3")
 HANDLE_LIBCALL(UREM_I128, "__umodti3")
 HANDLE_LIBCALL(SDIVREM_I8, nullptr)
 HANDLE_LIBCALL(SDIVREM_I16, nullptr)
-HANDLE_LIBCALL(SDIVREM_I32, nullptr)
-HANDLE_LIBCALL(SDIVREM_I64, nullptr)
-HANDLE_LIBCALL(SDIVREM_I128, nullptr)
+HANDLE_LIBCALL(SDIVREM_I32, "__divmodsi4")
+HANDLE_LIBCALL(SDIVREM_I64, "__divmoddi4")
+HANDLE_LIBCALL(SDIVREM_I128, "__divmodti4")
 HANDLE_LIBCALL(UDIVREM_I8, nullptr)
 HANDLE_LIBCALL(UDIVREM_I16, nullptr)
-HANDLE_LIBCALL(UDIVREM_I32, nullptr)
-HANDLE_LIBCALL(UDIVREM_I64, nullptr)
-HANDLE_LIBCALL(UDIVREM_I128, nullptr)
+HANDLE_LIBCALL(UDIVREM_I32, "__udivmodsi4")
+HANDLE_LIBCALL(UDIVREM_I64, "__udivmoddi4")
+HANDLE_LIBCALL(UDIVREM_I128, "__udivmodti4")
 HANDLE_LIBCALL(NEG_I32, "__negsi2")
 HANDLE_LIBCALL(NEG_I64, "__negdi2")
+HANDLE_LIBCALL(NEG_I128, "__negti2")
 HANDLE_LIBCALL(CTLZ_I32, "__clzsi2")
 HANDLE_LIBCALL(CTLZ_I64, "__clzdi2")
 HANDLE_LIBCALL(CTLZ_I128, "__clzti2")
+HANDLE_LIBCALL(CTTZ_I32, "__ctzsi2")
+HANDLE_LIBCALL(CTTZ_I64, "__ctzdi2")
+HANDLE_LIBCALL(CTTZ_I128, "__ctzti2")
+HANDLE_LIBCALL(FFS_I32, "__ffssi2")
+HANDLE_LIBCALL(FFS_I64, "__ffsdi2")
+HANDLE_LIBCALL(FFS_I128, "__ffsti2")
+HANDLE_LIBCALL(PARITY_I32, "__paritysi2")
+HANDLE_LIBCALL(PARITY_I64, "__paritydi2")
+HANDLE_LIBCALL(PARITY_I128, "__parityti2")
+HANDLE_LIBCALL(POPCOUNT_I32, "__popcountsi2")
+HANDLE_LIBCALL(POPCOUNT_I64, "__popcountdi2")
+HANDLE_LIBCALL(POPCOUNT_I128, "__popcountti2")
+HANDLE_LIBCALL(BSWAP_I32, "__bswapsi2")
+HANDLE_LIBCALL(BSWAP_I64, "__bswapdi2")
+HANDLE_LIBCALL(ABSV_I32, "__absvsi2")
+HANDLE_LIBCALL(ABSV_I64, "__absvdi2")
+HANDLE_LIBCALL(ABSV_I128, "__absvti2")
+HANDLE_LIBCALL(NEGV_I32, "__negvsi2")
+HANDLE_LIBCALL(NEGV_I64, "__negvdi2")
+HANDLE_LIBCALL(NEGV_I128, "__negvti2")
+HANDLE_LIBCALL(ADDV_I32, "__addvsi3")
+HANDLE_LIBCALL(ADDV_I64, "__addvdi3")
+HANDLE_LIBCALL(ADDV_I128, "__addvti3")
+HANDLE_LIBCALL(SUBV_I32, "__subvsi3")
+HANDLE_LIBCALL(SUBV_I64, "__subvdi3")
+HANDLE_LIBCALL(SUBV_I128, "__subvti3")
+HANDLE_LIBCALL(MULV_I32, "__mulvsi3")
+HANDLE_LIBCALL(MULV_I64, "__mulvdi3")
+HANDLE_LIBCALL(MULV_I128, "__mulvti3")
+HANDLE_LIBCALL(CMP_I64, "__cmpdi2")
+HANDLE_LIBCALL(CMP_I128, "__cmpti2")
+HANDLE_LIBCALL(UCMP_I64, "__ucmpdi2")
+HANDLE_LIBCALL(UCMP_I128, "__ucmpti2")
 
 // Floating-point
 HANDLE_LIBCALL(ADD_F32, "__addsf3")
@@ -101,11 +135,19 @@ HANDLE_LIBCALL(MUL_F32, "__mulsf3")
 HANDLE_LIBCALL(MUL_F64, "__muldf3")
 HANDLE_LIBCALL(MUL_F80, "__mulxf3")
 HANDLE_LIBCALL(MUL_F128, "__multf3")
+HANDLE_LIBCALL(MULC_F32, "__mulsc3")
+HANDLE_LIBCALL(MULC_F64, "__muldc3")
+HANDLE_LIBCALL(MULC_F80, "__mulxc3")
+HANDLE_LIBCALL(MULC_F128, "__multc3")
 HANDLE_LIBCALL(MUL_PPCF128, "__gcc_qmul")
 HANDLE_LIBCALL(DIV_F32, "__divsf3")
 HANDLE_LIBCALL(DIV_F64, "__divdf3")
 HANDLE_LIBCALL(DIV_F80, "__divxf3")
 HANDLE_LIBCALL(DIV_F128, "__divtf3")
+HANDLE_LIBCALL(DIVC_F32, "__divsc3")
+HANDLE_LIBCALL(DIVC_F64, "__divdc3")
+HANDLE_LIBCALL(DIVC_F80, "__divxc3")
+HANDLE_LIBCALL(DIVC_F128, "__divtc3")
 HANDLE_LIBCALL(DIV_PPCF128, "__gcc_qdiv")
 HANDLE_LIBCALL(REM_F32, "fmodf")
 HANDLE_LIBCALL(REM_F64, "fmod")
@@ -434,6 +476,41 @@ HANDLE_LIBCALL(UO_F64, "__unorddf2")
 HANDLE_LIBCALL(UO_F128, "__unordtf2")
 HANDLE_LIBCALL(UO_PPCF128, "__gcc_qunord")
 
+// VFP
+HANDLE_LIBCALL(VFP_ADD_F32, "__addsf3vfp")
+HANDLE_LIBCALL(VFP_ADD_F64, "__adddf3vfp")
+HANDLE_LIBCALL(VFP_DIV_F32, "__divsf3vfp")
+HANDLE_LIBCALL(VFP_DIV_F64, "__divdf3vfp")
+HANDLE_LIBCALL(VFP_OEQ_F32, "__eqsf2vfp")
+HANDLE_LIBCALL(VFP_OEQ_F64, "__eqdf2vfp")
+HANDLE_LIBCALL(VFP_FPEXT_F32_F64, "__extendsfdf2vfp")
+HANDLE_LIBCALL(VFP_FPTOSINT_F64_I32, "__fixdfsivfp")
+HANDLE_LIBCALL(VFP_FPTOSINT_F32_I32, "__fixsfsivfp")
+HANDLE_LIBCALL(VFP_FPTOUINT_F32_I32, "__fixunssfsivfp")
+HANDLE_LIBCALL(VFP_FPTOUINT_F64_I32, "__fixunsdfsivfp")
+HANDLE_LIBCALL(VFP_SINTTOFP_I32_F64, "__floatsidfvfp")
+HANDLE_LIBCALL(VFP_SINTTOFP_I32_F32, "__floatsisfvfp")
+HANDLE_LIBCALL(VFP_UINTTOFP_I32_F64, "__floatunssidfvfp")
+HANDLE_LIBCALL(VFP_UINTTOFP_I32_F32, "__floatunssisfvfp")
+HANDLE_LIBCALL(VFP_OGE_F64, "__gedf2vfp")
+HANDLE_LIBCALL(VFP_OGE_F32, "__gesf2vfp")
+HANDLE_LIBCALL(VFP_OGT_F64, "__gtdf2vfp")
+HANDLE_LIBCALL(VFP_OGT_F32, "__gtsf2vfp")
+HANDLE_LIBCALL(VFP_OLE_F64, "__ledf2vfp")
+HANDLE_LIBCALL(VFP_OLE_F32, "__lesf2vfp")
+HANDLE_LIBCALL(VFP_OLT_F64, "__ltdf2vfp")
+HANDLE_LIBCALL(VFP_OLT_F32, "__ltsf2vfp")
+HANDLE_LIBCALL(VFP_MUL_F64, "__muldf3vfp")
+HANDLE_LIBCALL(VFP_MUL_F32, "__mulsf3vfp")
+HANDLE_LIBCALL(VFP_UNE_F64, "__nedf2vfp")
+HANDLE_LIBCALL(VFP_NEG_F64, "__negdf2vfp")
+HANDLE_LIBCALL(VFP_NEG_F32, "__negsf2vfp")
+HANDLE_LIBCALL(VFP_SUB_F64, "__subdf3vfp")
+HANDLE_LIBCALL(VFP_SUB_F32, "__subsf3vfp")
+HANDLE_LIBCALL(VFP_FPROUND_F64_F32, "__truncdfsf2vfp")
+HANDLE_LIBCALL(VFP_UO_F64, "__unorddf2vfp")
+HANDLE_LIBCALL(VFP_UO_F32, "__unordsf2vfp")
+
 // Memory
 HANDLE_LIBCALL(MEMCPY, "memcpy")
 HANDLE_LIBCALL(MEMMOVE, "memmove")
diff --git a/llvm/lib/Transforms/IPO/Internalize.cpp b/llvm/lib/Transforms/IPO/Internalize.cpp
index 0b8fde6489f8e76..2539699103bc056 100644
--- a/llvm/lib/Transforms/IPO/Internalize.cpp
+++ b/llvm/lib/Transforms/IPO/Internalize.cpp
@@ -51,6 +51,12 @@ static cl::list<std::string>
     APIList("internalize-public-api-list", cl::value_desc("list"),
             cl::desc("A list of symbol names to preserve"), cl::CommaSeparated);
 
+static const char *PreservedLibcallSymbols[] = {
+#define HANDLE_LIBCALL(code, name) name,
+#include "llvm/IR/RuntimeLibcalls.def"
+#undef HANDLE_LIBCALL
+};
+
 namespace {
 // Helper to load an API list to preserve from file and expose it as a functor
 // for internalization.
@@ -127,6 +133,10 @@ bool InternalizePass::shouldPreserveGV(const GlobalValue &GV) {
   if (AlwaysPreserved.count(GV.getName()))
     return true;
 
+  // Preserve built-in functions
+  if (llvm::is_contained(PreservedLibcallSymbols, GV.getName()))
+    return true;
+
   return MustPreserveGV(GV);
 }
 
diff --git a/llvm/test/Transforms/Internalize/built-in.ll b/llvm/test/Transforms/Internalize/built-in.ll
new file mode 100644
index 000000000000000..e9aea0837098a78
--- /dev/null
+++ b/llvm/test/Transforms/Internalize/built-in.ll
@@ -0,0 +1,18 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 3
+; RUN: opt < %s -passes=internalize -S | FileCheck %s
+
+define i128 @__multi3(i128 %0, i128 %1) {
+; CHECK-LABEL: define i128 @__multi3(
+; CHECK-SAME: i128 [[TMP0:%.*]], i128 [[TMP1:%.*]]) {
+; CHECK-NEXT:    ret i128 0
+;
+  ret i128 0
+}
+
+define i64 @__udivmoddi4(i64 %0, i64 %1, ptr %2) {
+; CHECK-LABEL: define i64 @__udivmoddi4(
+; CHECK-SAME: i64 [[TMP0:%.*]], i64 [[TMP1:%.*]], ptr [[TMP2:%.*]]) {
+; CHECK-NEXT:    ret i64 0
+;
+  ret i64 0
+}

Copy link
Contributor

@teresajohnson teresajohnson left a comment

Choose a reason for hiding this comment

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

Can you add some more description of the motivation? Specifically, why doesn't the earlier referenced patch (e06bac4) address this issue (for the existing list of runtime libcalls)?

Also, this patch seems to have 2 aspects, where the second is to expand the list of runtime libcalls. Can you give some more background on that aspect of the change?

@DianQK
Copy link
Member Author

DianQK commented Oct 17, 2023

Can you add some more description of the motivation? Specifically, why doesn't the earlier referenced patch (e06bac4) address this issue (for the existing list of runtime libcalls)?

This is because rust calls the internalizeModule method directly when implementing LTO. https://github.com/rust-lang/rust/blob/94ba57cef4987cfc1aee063ff459bcd2ff2f3fab/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp#L1146
So I think it's appropriate to add a similar rule here. In fact here are some similar rules I mentioned in the issue. For example __ssp_canary_word.

Also, this patch seems to have 2 aspects, where the second is to expand the list of runtime libcalls. Can you give some more background on that aspect of the change?

Newly added built-in functions are also internalized as a result of participation in LTO.
I'm sorry I'm not really sure what additional impact the extended list will have.

@teresajohnson
Copy link
Contributor

Can you add some more description of the motivation? Specifically, why doesn't the earlier referenced patch (e06bac4) address this issue (for the existing list of runtime libcalls)?

This is because rust calls the internalizeModule method directly when implementing LTO. https://github.com/rust-lang/rust/blob/94ba57cef4987cfc1aee063ff459bcd2ff2f3fab/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp#L1146 So I think it's appropriate to add a similar rule here. In fact here are some similar rules I mentioned in the issue. For example __ssp_canary_word.

Is it possible to perform this in the Rust code (e.g. I see it already adds an asan internal symbol to the preserved symbols checked in the callback it passes to internalizeModule)? My concern is that for LLVM LTO we have already done all this checking, and adding an extra search through the list of builtins for each symbol is inefficient.

Also, this patch seems to have 2 aspects, where the second is to expand the list of runtime libcalls. Can you give some more background on that aspect of the change?

Newly added built-in functions are also internalized as a result of participation in LTO. I'm sorry I'm not really sure what additional impact the extended list will have.

I'd suggest splitting this part into its own patch since it is orthogonal to the main functionality change (although as mentioned above I would prefer the checking be moved into the caller to avoid duplication).

@DianQK
Copy link
Member Author

DianQK commented Oct 25, 2023

Can you add some more description of the motivation? Specifically, why doesn't the earlier referenced patch (e06bac4) address this issue (for the existing list of runtime libcalls)?

This is because rust calls the internalizeModule method directly when implementing LTO. https://github.com/rust-lang/rust/blob/94ba57cef4987cfc1aee063ff459bcd2ff2f3fab/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp#L1146 So I think it's appropriate to add a similar rule here. In fact here are some similar rules I mentioned in the issue. For example __ssp_canary_word.

Is it possible to perform this in the Rust code (e.g. I see it already adds an asan internal symbol to the preserved symbols checked in the callback it passes to internalizeModule)? My concern is that for LLVM LTO we have already done all this checking, and adding an extra search through the list of builtins for each symbol is inefficient.

Thank you for responding.
Yes, it can. I found a way to do this without having to manually maintain the list at rust-lang/rust@b592f29.
I think that could work. I'll leave this approach in the issue when Rust's PRs merge.

@DianQK DianQK closed this Oct 25, 2023
@DianQK DianQK deleted the internalize-built-in branch December 12, 2023 14:17
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.

InternalizePass should consider built-in functions.
3 participants