[llvm] [SDAG] Avoid creating redundant stack slots when lowering FSINCOS (PR #108401)

Benjamin Maxwell via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 23 06:56:30 PDT 2024


================
@@ -2355,68 +2347,80 @@ static bool useSinCos(SDNode *Node) {
 }
 
 /// Issue libcalls to sincos to compute sin / cos pairs.
-void
-SelectionDAGLegalize::ExpandSinCosLibCall(SDNode *Node,
-                                          SmallVectorImpl<SDValue> &Results) {
-  RTLIB::Libcall LC;
-  switch (Node->getSimpleValueType(0).SimpleTy) {
-  default: llvm_unreachable("Unexpected request for libcall!");
-  case MVT::f32:     LC = RTLIB::SINCOS_F32; break;
-  case MVT::f64:     LC = RTLIB::SINCOS_F64; break;
-  case MVT::f80:     LC = RTLIB::SINCOS_F80; break;
-  case MVT::f128:    LC = RTLIB::SINCOS_F128; break;
-  case MVT::ppcf128: LC = RTLIB::SINCOS_PPCF128; break;
+void SelectionDAGLegalize::ExpandSinCosLibCall(
+    SDNode *Node, SmallVectorImpl<SDValue> &Results) {
+  EVT VT = Node->getValueType(0);
+  Type *Ty = VT.getTypeForEVT(*DAG.getContext());
+  RTLIB::Libcall LC = RTLIB::getFSINCOS(VT);
+
+  // Find users of the node that store the results. The destination pointers
+  // can be used instead of creating stack allocations.
+  std::array<StoreSDNode *, 2> ResultStores = {nullptr};
+  for (SDNode *User : Node->uses()) {
+    if (!ISD::isNormalStore(User))
+      continue;
+    auto *ST = cast<StoreSDNode>(User);
+    if (!ST->isSimple() || ST->getAddressSpace() != 0 ||
+        ST->getAlign() < DAG.getDataLayout().getABITypeAlign(Ty))
+      continue;
----------------
MacDue wrote:

I realized the only case this is fully safe is when the two input chains are the same (and the chain does not depend on fsincos).  If the chains are not the same the stores may alias, and the result could be different depending on the order of stores in the sincos call (which as far as I can tell is not defined). 

I've added these restrictions and updated the tests. Note: Even when things may alias we're still able to replace one of the stores (so there's still some benefit). 

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


More information about the llvm-commits mailing list