[llvm] [LLVM] Slay undead copysign code (PR #111269)

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 14 05:35:05 PDT 2024


================
@@ -1649,12 +1649,19 @@ 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);
+
+  assert(N->getValueType(0) == MVT::ppcf128 &&
+         "Logic only correct for ppcf128!");
+  SDLoc DL = SDLoc(N);
+  SDValue Tmp = SDValue();
+  GetExpandedFloat(N->getOperand(0), Lo, Tmp);
+
+  Hi = DAG.getNode(ISD::FCOPYSIGN, DL, Tmp.getValueType(), Tmp,
+                   N->getOperand(1));
+  // a double-double is Hi + Lo, so if Hi flips sign, so must Lo
+  Lo = DAG.getSelectCC(DL, Tmp, Hi, Lo,
+                       DAG.getNode(ISD::FNEG, DL, Lo.getValueType(), Lo),
+                       ISD::SETEQ);
----------------
arsenm wrote:

copysign needs to respect the sign bit of incoming nan values. This logic will flip the sign bit for Lo for a nan Hi. I don't really understand how ppcf128 is supposed to work for nans, but I would expect this logic to be inverted (e.g. `Hi UEQ fabs(hi) ? Lo : neg(Lo)`)

I guess it's ok for fabs, or fabs is wrong? 

https://github.com/llvm/llvm-project/pull/111269


More information about the llvm-commits mailing list