[SDAG] Add missing ppc_fp128 ExpandFloatRes for sincos[pi]

[MacDue]

No description provided.

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-powerpc

Author: Benjamin Maxwell (MacDue)

Changes

---

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

3 Files Affected:

• (modified) llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp (+11)
• (modified) llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h (+2)
• (added) llvm/test/CodeGen/PowerPC/llvm.sincos.ll (+51)

diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
index 0244c170a2123..9fbcb5bc31537 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
@@ -1570,6 +1570,8 @@ void DAGTypeLegalizer::ExpandFloatResult(SDNode *N, unsigned ResNo) {
   case ISD::STRICT_FREM:
   case ISD::FREM:       ExpandFloatRes_FREM(N, Lo, Hi); break;
   case ISD::FMODF:   ExpandFloatRes_FMODF(N); break;
+  case ISD::FSINCOS: ExpandFloatRes_FSINCOS(N); break;
+  case ISD::FSINCOSPI: ExpandFloatRes_FSINCOSPI(N); break;
     // clang-format on
   }
 
@@ -1625,6 +1627,15 @@ void DAGTypeLegalizer::ExpandFloatRes_FMODF(SDNode *N) {
                                        /*CallRetResNo=*/0);
 }
 
+void DAGTypeLegalizer::ExpandFloatRes_FSINCOS(SDNode *N) {
+  ExpandFloatRes_UnaryWithTwoFPResults(N, RTLIB::getSINCOS(N->getValueType(0)));
+}
+
+void DAGTypeLegalizer::ExpandFloatRes_FSINCOSPI(SDNode *N) {
+  ExpandFloatRes_UnaryWithTwoFPResults(N,
+                                       RTLIB::getSINCOSPI(N->getValueType(0)));
+}
+
 void DAGTypeLegalizer::ExpandFloatRes_UnaryWithTwoFPResults(
     SDNode *N, RTLIB::Libcall LC, std::optional<unsigned> CallRetResNo) {
   assert(!N->isStrictFPOpcode() && "strictfp not implemented");
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
index cac969f7e2185..74d7210743372 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
@@ -718,6 +718,8 @@ class LLVM_LIBRARY_VISIBILITY DAGTypeLegalizer {
   void ExpandFloatRes_LOAD      (SDNode *N, SDValue &Lo, SDValue &Hi);
   void ExpandFloatRes_XINT_TO_FP(SDNode *N, SDValue &Lo, SDValue &Hi);
   void ExpandFloatRes_FMODF(SDNode *N);
+  void ExpandFloatRes_FSINCOS(SDNode* N);
+  void ExpandFloatRes_FSINCOSPI(SDNode* N);
   // clang-format on
 
   // Float Operand Expansion.
diff --git a/llvm/test/CodeGen/PowerPC/llvm.sincos.ll b/llvm/test/CodeGen/PowerPC/llvm.sincos.ll
new file mode 100644
index 0000000000000..80cfaf2d17791
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/llvm.sincos.ll
@@ -0,0 +1,51 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
+; RUN: llc -mcpu=pwr9 -mtriple=powerpc64le-gnu-linux \
+; RUN:   -ppc-vsr-nums-as-vr -ppc-asm-full-reg-names < %s | FileCheck %s
+
+define { ppc_fp128, ppc_fp128 } @test_sincos_ppcf128(ppc_fp128 %a) {
+; CHECK-LABEL: test_sincos_ppcf128:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    mflr r0
+; CHECK-NEXT:    stdu r1, -64(r1)
+; CHECK-NEXT:    std r0, 80(r1)
+; CHECK-NEXT:    .cfi_def_cfa_offset 64
+; CHECK-NEXT:    .cfi_offset lr, 16
+; CHECK-NEXT:    addi r5, r1, 48
+; CHECK-NEXT:    addi r6, r1, 32
+; CHECK-NEXT:    bl sincosl
+; CHECK-NEXT:    nop
+; CHECK-NEXT:    lfd f1, 48(r1)
+; CHECK-NEXT:    lfd f2, 56(r1)
+; CHECK-NEXT:    lfd f3, 32(r1)
+; CHECK-NEXT:    lfd f4, 40(r1)
+; CHECK-NEXT:    addi r1, r1, 64
+; CHECK-NEXT:    ld r0, 16(r1)
+; CHECK-NEXT:    mtlr r0
+; CHECK-NEXT:    blr
+  %result = call { ppc_fp128, ppc_fp128 } @llvm.sincos.ppcf128(ppc_fp128 %a)
+  ret { ppc_fp128, ppc_fp128 } %result
+}
+
+define { ppc_fp128, ppc_fp128 } @test_sincospi_ppcf128(ppc_fp128 %a) {
+; CHECK-LABEL: test_sincospi_ppcf128:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    mflr r0
+; CHECK-NEXT:    stdu r1, -64(r1)
+; CHECK-NEXT:    std r0, 80(r1)
+; CHECK-NEXT:    .cfi_def_cfa_offset 64
+; CHECK-NEXT:    .cfi_offset lr, 16
+; CHECK-NEXT:    addi r5, r1, 48
+; CHECK-NEXT:    addi r6, r1, 32
+; CHECK-NEXT:    bl sincospil
+; CHECK-NEXT:    nop
+; CHECK-NEXT:    lfd f1, 48(r1)
+; CHECK-NEXT:    lfd f2, 56(r1)
+; CHECK-NEXT:    lfd f3, 32(r1)
+; CHECK-NEXT:    lfd f4, 40(r1)
+; CHECK-NEXT:    addi r1, r1, 64
+; CHECK-NEXT:    ld r0, 16(r1)
+; CHECK-NEXT:    mtlr r0
+; CHECK-NEXT:    blr
+  %result = call { ppc_fp128, ppc_fp128 } @llvm.sincospi.ppcf128(ppc_fp128 %a)
+  ret { ppc_fp128, ppc_fp128 } %result
+}

[arsenm]

I don't think this can be recognized as a tail call, the return needs to be the immediate next instruction after the call

[MacDue]

the return needs to be the immediate next instruction after the call

That is what codegen emits, the stores and extractvalues fold away to nothing, though the tail call logic does not recognise this.

[arsenm]

What do you mean by "what the codegen emits"? The issue is what is in the IR. This function should return the original { ppc_fp128, ppc_fp128 } with no extracts or stores

[MacDue]

Right, but returning a struct would never result in a tail call for the standard expansion.
This IR does result in something that codegen could recognise as a tail call (and there's logic in SDAG that could do this), but right now it fails to do so. Look at the current codegen:

; CHECK-LABEL: test_sincos_ppcf128_tail_call:
; CHECK:       # %bb.0:
<frame setup>
; CHECK-NEXT:    mflr r0
; CHECK-NEXT:    stdu r1, -32(r1)
; CHECK-NEXT:    std r0, 48(r1)
; CHECK-NEXT:    .cfi_def_cfa_offset 32
; CHECK-NEXT:    .cfi_offset lr, 16

<call to sincos>
; CHECK-NEXT:    bl sincosl

<frame destruction> 
; CHECK-NEXT:    nop
; CHECK-NEXT:    addi r1, r1, 32
; CHECK-NEXT:    ld r0, 16(r1)
; CHECK-NEXT:    mtlr r0
; CHECK-NEXT:    blr

This could simply be a jump to sincosl.

[arsenm]

This should return the raw structure type

[MacDue]

That won't result in something that could be a tail call on any target that uses the (semi)-standard GNU sincos function (since it'll need to emit loads after the call).

[arsenm]

I would still test both forms of tail call, it will go down different paths

[MacDue]

I would still test both forms of tail call, it will go down different paths

Sure, done 👍