[llvm] [LLVM] Slay undead copysign code (PR #111269)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Oct 5 17:51:01 PDT 2024
https://github.com/workingjubilee created https://github.com/llvm/llvm-project/pull/111269
Frontends that emit LLVMIR would like to be able to count on float operations that can be encoded as simple bit-operations as lowering to machine-code routines embedded into the binary. This is desired so that they do not affect what one might think of as "freestanding" binaries. GlobalISel explicitly handles this lowering for copysign[^1].
Perhaps it is just my newness to the codebase but I was unable to find a case where SelectionDAG likewise guaranteed that copysign was lowered to machine code. Instead, it seemed to be, at least in theory, prone to lowering to runtime libcalls, as indicated by its entry in llvm/include/llvm/IR/RuntimeLibcalls.def[^2]!
In removing this code, it may be necessary to preserve runtime libcalls only for architecture-specific variants of these float operations that may have architecture-specific semantics. However, ordinary IEEE754 floats have nonesuch, and these bitops are then guaranteed to be bitops.
As no tests seem to fail after doing so, it seems plausible to me this was actually-dead code that was only in-theory reachable. If that is so, purge it from the codebase, so this revenant may plague us no more.
[^1]: https://github.com/llvm/llvm-project/blob/46944b0cbc9a9d8daad0182c40fcd3560bc9ca35/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp#L7772-L7811
[^2]: https://github.com/llvm/llvm-project/blob/46944b0cbc9a9d8daad0182c40fcd3560bc9ca35/llvm/include/llvm/IR/RuntimeLibcalls.def#L287-L291
>From 7c0b8b2ddb39f42d443c021a0999c26dbcd4e46f Mon Sep 17 00:00:00 2001
From: Jubilee Young <workingjubilee at gmail.com>
Date: Sat, 5 Oct 2024 15:32:00 -0700
Subject: [PATCH 1/4] [Codegen] Avoid UB-incurring `llvm_unreachable` in
IntrinsicLowering
There is no proof this improves performance and evidence it does not,
so choose a consistent and more boring assert-then-break.
---
llvm/lib/CodeGen/IntrinsicLowering.cpp | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/CodeGen/IntrinsicLowering.cpp b/llvm/lib/CodeGen/IntrinsicLowering.cpp
index 256c081b46e262..3df8b4524f9a2a 100644
--- a/llvm/lib/CodeGen/IntrinsicLowering.cpp
+++ b/llvm/lib/CodeGen/IntrinsicLowering.cpp
@@ -58,7 +58,9 @@ static Value *LowerBSWAP(LLVMContext &Context, Value *V, Instruction *IP) {
IRBuilder<> Builder(IP);
switch(BitSize) {
- default: llvm_unreachable("Unhandled type size of value to byteswap!");
+ default:
+ assert("Unhandled type size of value to byteswap!");
+ break;
case 16: {
Value *Tmp1 = Builder.CreateShl(V, ConstantInt::get(V->getType(), 8),
"bswap.2");
@@ -203,7 +205,9 @@ static void ReplaceFPIntrinsicWithCall(CallInst *CI, const char *Fname,
const char *Dname,
const char *LDname) {
switch (CI->getArgOperand(0)->getType()->getTypeID()) {
- default: llvm_unreachable("Invalid type in intrinsic");
+ default:
+ assert("Invalid type in intrinsic");
+ break;
case Type::FloatTyID:
ReplaceCallWith(Fname, CI, CI->arg_begin(), CI->arg_end(),
Type::getFloatTy(CI->getContext()));
>From d03f2da1eee980fd4094d4236f2e8d82abe47073 Mon Sep 17 00:00:00 2001
From: Jubilee Young <workingjubilee at gmail.com>
Date: Sat, 5 Oct 2024 15:42:49 -0700
Subject: [PATCH 2/4] [Codegen] Demand llvm.copysign.f{16,32,64,128} lowers
without libcalls
---
llvm/lib/CodeGen/IntrinsicLowering.cpp | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/CodeGen/IntrinsicLowering.cpp b/llvm/lib/CodeGen/IntrinsicLowering.cpp
index 3df8b4524f9a2a..61d5f75273fa2b 100644
--- a/llvm/lib/CodeGen/IntrinsicLowering.cpp
+++ b/llvm/lib/CodeGen/IntrinsicLowering.cpp
@@ -442,7 +442,15 @@ void IntrinsicLowering::LowerIntrinsicCall(CallInst *CI) {
break;
}
case Intrinsic::copysign: {
- ReplaceFPIntrinsicWithCall(CI, "copysignf", "copysign", "copysignl");
+ switch (CI->getArgOperand(0)->getType()->getTypeID()) {
+ default:
+ assert("only need a copysign libcall for arch-specific floats");
+ break;
+ case Type::X86_FP80TyID:
+ case Type::PPC_FP128TyID:
+ ReplaceCallWith("copysignl", CI, CI->arg_begin(), CI->arg_end(),
+ Type::getFloatTy(CI->getContext()));
+ }
break;
}
case Intrinsic::get_rounding:
>From 937f1be27971207ed75590da87a6db12bb62d704 Mon Sep 17 00:00:00 2001
From: Jubilee Young <workingjubilee at gmail.com>
Date: Sat, 5 Oct 2024 15:47:58 -0700
Subject: [PATCH 3/4] [IR] Remove COPYSIGN_F{32,64,128} from runtime libcalls
---
llvm/include/llvm/IR/RuntimeLibcalls.def | 3 ---
llvm/lib/IR/RuntimeLibcalls.cpp | 1 -
llvm/lib/Target/SystemZ/ZOSLibcallNames.def | 3 ---
.../Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp | 3 ---
4 files changed, 10 deletions(-)
diff --git a/llvm/include/llvm/IR/RuntimeLibcalls.def b/llvm/include/llvm/IR/RuntimeLibcalls.def
index 69cf43140ad4bd..b891dce07d60dd 100644
--- a/llvm/include/llvm/IR/RuntimeLibcalls.def
+++ b/llvm/include/llvm/IR/RuntimeLibcalls.def
@@ -284,10 +284,7 @@ HANDLE_LIBCALL(FLOOR_F64, "floor")
HANDLE_LIBCALL(FLOOR_F80, "floorl")
HANDLE_LIBCALL(FLOOR_F128, "floorl")
HANDLE_LIBCALL(FLOOR_PPCF128, "floorl")
-HANDLE_LIBCALL(COPYSIGN_F32, "copysignf")
-HANDLE_LIBCALL(COPYSIGN_F64, "copysign")
HANDLE_LIBCALL(COPYSIGN_F80, "copysignl")
-HANDLE_LIBCALL(COPYSIGN_F128, "copysignl")
HANDLE_LIBCALL(COPYSIGN_PPCF128, "copysignl")
HANDLE_LIBCALL(FMIN_F32, "fminf")
HANDLE_LIBCALL(FMIN_F64, "fmin")
diff --git a/llvm/lib/IR/RuntimeLibcalls.cpp b/llvm/lib/IR/RuntimeLibcalls.cpp
index d806f8093459ee..ca78892ec78838 100644
--- a/llvm/lib/IR/RuntimeLibcalls.cpp
+++ b/llvm/lib/IR/RuntimeLibcalls.cpp
@@ -61,7 +61,6 @@ void RuntimeLibcallsInfo::initLibcalls(const Triple &TT) {
setLibcallName(RTLIB::ROUND_F128, "roundf128");
setLibcallName(RTLIB::ROUNDEVEN_F128, "roundevenf128");
setLibcallName(RTLIB::FLOOR_F128, "floorf128");
- setLibcallName(RTLIB::COPYSIGN_F128, "copysignf128");
setLibcallName(RTLIB::FMIN_F128, "fminf128");
setLibcallName(RTLIB::FMAX_F128, "fmaxf128");
setLibcallName(RTLIB::LROUND_F128, "lroundf128");
diff --git a/llvm/lib/Target/SystemZ/ZOSLibcallNames.def b/llvm/lib/Target/SystemZ/ZOSLibcallNames.def
index 12a01522a7e643..a53c9618696fcc 100644
--- a/llvm/lib/Target/SystemZ/ZOSLibcallNames.def
+++ b/llvm/lib/Target/SystemZ/ZOSLibcallNames.def
@@ -87,9 +87,6 @@ HANDLE_LIBCALL(EXP2_F128, "@@LXP2 at B")
HANDLE_LIBCALL(COS_F64, "@@SCOS at B")
HANDLE_LIBCALL(COS_F32, "@@FCOS at B")
HANDLE_LIBCALL(COS_F128, "@@LCOS at B")
-HANDLE_LIBCALL(COPYSIGN_F64, "@@DCPY at B")
-HANDLE_LIBCALL(COPYSIGN_F32, "@@FCPY at B")
-HANDLE_LIBCALL(COPYSIGN_F128, "@@LCPY at B")
HANDLE_LIBCALL(CEIL_F64, "@@SCEL at B")
HANDLE_LIBCALL(CEIL_F32, "@@FCEL at B")
HANDLE_LIBCALL(CEIL_F128, "@@LCEL at B")
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
index ba3ab5164af267..13c8a6fe1524fc 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyRuntimeLibcallSignatures.cpp
@@ -261,9 +261,6 @@ struct RuntimeLibcallSignatureTable {
Table[RTLIB::FLOOR_F32] = f32_func_f32;
Table[RTLIB::FLOOR_F64] = f64_func_f64;
Table[RTLIB::FLOOR_F128] = i64_i64_func_i64_i64;
- Table[RTLIB::COPYSIGN_F32] = f32_func_f32_f32;
- Table[RTLIB::COPYSIGN_F64] = f64_func_f64_f64;
- Table[RTLIB::COPYSIGN_F128] = i64_i64_func_i64_i64_i64_i64;
Table[RTLIB::FMIN_F32] = f32_func_f32_f32;
Table[RTLIB::FMIN_F64] = f64_func_f64_f64;
Table[RTLIB::FMIN_F128] = i64_i64_func_i64_i64_i64_i64;
>From 816d37e2caa2bb40db3a615410f26d4e42f10ae3 Mon Sep 17 00:00:00 2001
From: Jubilee Young <workingjubilee at gmail.com>
Date: Sat, 5 Oct 2024 17:06:00 -0700
Subject: [PATCH 4/4] [SelectionDAG] Only handle arch-specific COPYSIGNs as
libcalls
---
llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
index 2c81c829e75cbb..0e4d364f69f216 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
@@ -1649,12 +1649,11 @@ void DAGTypeLegalizer::ExpandFloatRes_FCEIL(SDNode *N,
void DAGTypeLegalizer::ExpandFloatRes_FCOPYSIGN(SDNode *N,
SDValue &Lo, SDValue &Hi) {
- ExpandFloatRes_Binary(N, GetFPLibCall(N->getValueType(0),
- RTLIB::COPYSIGN_F32,
- RTLIB::COPYSIGN_F64,
- RTLIB::COPYSIGN_F80,
- RTLIB::COPYSIGN_F128,
- RTLIB::COPYSIGN_PPCF128), Lo, Hi);
+
+ auto VT = N->getValueType(0);
+ ExpandFloatRes_Binary(N, (VT == MVT::f80 ? RTLIB::COPYSIGN_F80 :
+ VT == MVT::ppcf128 ? RTLIB::COPYSIGN_PPCF128 :
+ RTLIB::UNKNOWN_LIBCALL), Lo, Hi);
}
void DAGTypeLegalizer::ExpandFloatRes_FCOS(SDNode *N,
More information about the llvm-commits
mailing list