[PowerPC][CodeGen] Expand ISD::AssertNoFPClass for ppc_fp128 #152357

amy-kwan · 2025-08-06T18:17:46Z

780054d added support for ISD::AssertNoFPClass.

This ISD node can be used with the ppc_fp128 type, which is really just two f64s and requires expanding when used with ISD::AssertNoFPClass. Without the support for expanding the result, we get an assertion because the legalizer does not know how to expand the results of ppc_fp128 with ISD::AssertNoFPClass.

ExpandFloatResult #0: t7: ppcf128 = AssertNoFPClass t5, TargetConstant:i32<3>

LLVM ERROR: Do not know how to expand the result of this operator!

Thus, this patch aims to add support for the expand so we no longer assert.

This fixes #151375.

780054d added support for `ISD::AssertNoFPClass`. This ISD node can be used with the `ppc_fp128` type, which is really just two `f64s` and requires expanding when used with `ISD::AssertNoFPClass`. Without the support for expanding the result, we get an assertion because the legalizer does not know how to expand the results of `ppc_fp128` with `ISD::AssertNoFPClass`. ``` ExpandFloatResult #0: t7: ppcf128 = AssertNoFPClass t5, TargetConstant:i32<3> LLVM ERROR: Do not know how to expand the result of this operator! ``` Thus, this patch aims to add support for the expand so we no longer assert. This fixes llvm#151375.

llvmbot · 2025-08-06T18:18:19Z

@llvm/pr-subscribers-backend-powerpc

Author: Amy Kwan (amy-kwan)

Changes

780054d added support for ISD::AssertNoFPClass.

This ISD node can be used with the ppc_fp128 type, which is really just two f64s and requires expanding when used with ISD::AssertNoFPClass. Without the support for expanding the result, we get an assertion because the legalizer does not know how to expand the results of ppc_fp128 with ISD::AssertNoFPClass.

ExpandFloatResult #<!-- -->0: t7: ppcf128 = AssertNoFPClass t5, TargetConstant:i32&lt;3&gt;

LLVM ERROR: Do not know how to expand the result of this operator!

Thus, this patch aims to add support for the expand so we no longer assert.

This fixes #151375.

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

3 Files Affected:

(modified) llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp (+9)
(modified) llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h (+1)
(added) llvm/test/CodeGen/PowerPC/nofpclass.ll (+13)

diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
index 2cad36eff9c88..8140c0e771a7b 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
@@ -1551,6 +1551,7 @@ void DAGTypeLegalizer::ExpandFloatResult(SDNode *N, unsigned ResNo) {
   case ISD::VAARG:              ExpandRes_VAARG(N, Lo, Hi); break;
 
   case ISD::ConstantFP: ExpandFloatRes_ConstantFP(N, Lo, Hi); break;
+  case ISD::AssertNoFPClass: ExpandFloatRes_AssertNoFPClass(N, Lo, Hi); break;
   case ISD::FABS:       ExpandFloatRes_FABS(N, Lo, Hi); break;
   case ISD::STRICT_FMINNUM:
   case ISD::FMINNUM:    ExpandFloatRes_FMINNUM(N, Lo, Hi); break;
@@ -1966,6 +1967,14 @@ void DAGTypeLegalizer::ExpandFloatRes_FNEG(SDNode *N, SDValue &Lo,
   Hi = DAG.getNode(ISD::FNEG, dl, Hi.getValueType(), Hi);
 }
 
+void DAGTypeLegalizer::ExpandFloatRes_AssertNoFPClass(SDNode *N, SDValue &Lo,
+                                                      SDValue &Hi) {
+  SDLoc dl(N);
+  GetExpandedFloat(N->getOperand(0), Lo, Hi);
+  Lo = DAG.getNode(ISD::AssertNoFPClass, dl, Lo.getValueType(), Lo);
+  Hi = DAG.getNode(ISD::AssertNoFPClass, dl, Hi.getValueType(), Hi);
+}
+
 void DAGTypeLegalizer::ExpandFloatRes_FP_EXTEND(SDNode *N, SDValue &Lo,
                                                 SDValue &Hi) {
   EVT NVT = TLI.getTypeToTransformTo(*DAG.getContext(), N->getValueType(0));
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
index 63544e63e1da1..33fa3012618b3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
@@ -681,6 +681,7 @@ class LLVM_LIBRARY_VISIBILITY DAGTypeLegalizer {
       SDNode *N, RTLIB::Libcall LC, std::optional<unsigned> CallRetResNo = {});
 
   // clang-format off
+  void ExpandFloatRes_AssertNoFPClass(SDNode *N, SDValue &Lo, SDValue &Hi);
   void ExpandFloatRes_FABS      (SDNode *N, SDValue &Lo, SDValue &Hi);
   void ExpandFloatRes_FACOS     (SDNode *N, SDValue &Lo, SDValue &Hi);
   void ExpandFloatRes_FASIN     (SDNode *N, SDValue &Lo, SDValue &Hi);
diff --git a/llvm/test/CodeGen/PowerPC/nofpclass.ll b/llvm/test/CodeGen/PowerPC/nofpclass.ll
new file mode 100644
index 0000000000000..444e62cbbcffc
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/nofpclass.ll
@@ -0,0 +1,13 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -verify-machineinstrs  -mtriple=powerpc64le-unknown-linux-gnu < %s \
+; RUN:   | FileCheck %s
+; RUN: llc -verify-machineinstrs  -mtriple=powerpc64-ibm-aix-xcoff < %s \
+; RUN:   | FileCheck %s
+
+define ppc_fp128 @f(ppc_fp128 nofpclass(nan) %s) {
+; CHECK-LABEL: f:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    blr
+entry:
+  ret ppc_fp128 %s
+}

llvmbot · 2025-08-06T18:18:20Z

@llvm/pr-subscribers-llvm-selectiondag

Author: Amy Kwan (amy-kwan)

Changes

780054d added support for ISD::AssertNoFPClass.

This ISD node can be used with the ppc_fp128 type, which is really just two f64s and requires expanding when used with ISD::AssertNoFPClass. Without the support for expanding the result, we get an assertion because the legalizer does not know how to expand the results of ppc_fp128 with ISD::AssertNoFPClass.

ExpandFloatResult #<!-- -->0: t7: ppcf128 = AssertNoFPClass t5, TargetConstant:i32&lt;3&gt;

LLVM ERROR: Do not know how to expand the result of this operator!

Thus, this patch aims to add support for the expand so we no longer assert.

This fixes #151375.

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

3 Files Affected:

(modified) llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp (+9)
(modified) llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h (+1)
(added) llvm/test/CodeGen/PowerPC/nofpclass.ll (+13)

diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
index 2cad36eff9c88..8140c0e771a7b 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp
@@ -1551,6 +1551,7 @@ void DAGTypeLegalizer::ExpandFloatResult(SDNode *N, unsigned ResNo) {
   case ISD::VAARG:              ExpandRes_VAARG(N, Lo, Hi); break;
 
   case ISD::ConstantFP: ExpandFloatRes_ConstantFP(N, Lo, Hi); break;
+  case ISD::AssertNoFPClass: ExpandFloatRes_AssertNoFPClass(N, Lo, Hi); break;
   case ISD::FABS:       ExpandFloatRes_FABS(N, Lo, Hi); break;
   case ISD::STRICT_FMINNUM:
   case ISD::FMINNUM:    ExpandFloatRes_FMINNUM(N, Lo, Hi); break;
@@ -1966,6 +1967,14 @@ void DAGTypeLegalizer::ExpandFloatRes_FNEG(SDNode *N, SDValue &Lo,
   Hi = DAG.getNode(ISD::FNEG, dl, Hi.getValueType(), Hi);
 }
 
+void DAGTypeLegalizer::ExpandFloatRes_AssertNoFPClass(SDNode *N, SDValue &Lo,
+                                                      SDValue &Hi) {
+  SDLoc dl(N);
+  GetExpandedFloat(N->getOperand(0), Lo, Hi);
+  Lo = DAG.getNode(ISD::AssertNoFPClass, dl, Lo.getValueType(), Lo);
+  Hi = DAG.getNode(ISD::AssertNoFPClass, dl, Hi.getValueType(), Hi);
+}
+
 void DAGTypeLegalizer::ExpandFloatRes_FP_EXTEND(SDNode *N, SDValue &Lo,
                                                 SDValue &Hi) {
   EVT NVT = TLI.getTypeToTransformTo(*DAG.getContext(), N->getValueType(0));
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
index 63544e63e1da1..33fa3012618b3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
@@ -681,6 +681,7 @@ class LLVM_LIBRARY_VISIBILITY DAGTypeLegalizer {
       SDNode *N, RTLIB::Libcall LC, std::optional<unsigned> CallRetResNo = {});
 
   // clang-format off
+  void ExpandFloatRes_AssertNoFPClass(SDNode *N, SDValue &Lo, SDValue &Hi);
   void ExpandFloatRes_FABS      (SDNode *N, SDValue &Lo, SDValue &Hi);
   void ExpandFloatRes_FACOS     (SDNode *N, SDValue &Lo, SDValue &Hi);
   void ExpandFloatRes_FASIN     (SDNode *N, SDValue &Lo, SDValue &Hi);
diff --git a/llvm/test/CodeGen/PowerPC/nofpclass.ll b/llvm/test/CodeGen/PowerPC/nofpclass.ll
new file mode 100644
index 0000000000000..444e62cbbcffc
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/nofpclass.ll
@@ -0,0 +1,13 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -verify-machineinstrs  -mtriple=powerpc64le-unknown-linux-gnu < %s \
+; RUN:   | FileCheck %s
+; RUN: llc -verify-machineinstrs  -mtriple=powerpc64-ibm-aix-xcoff < %s \
+; RUN:   | FileCheck %s
+
+define ppc_fp128 @f(ppc_fp128 nofpclass(nan) %s) {
+; CHECK-LABEL: f:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    blr
+entry:
+  ret ppc_fp128 %s
+}

amy-kwan · 2025-08-06T18:23:07Z

llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp

@@ -1966,6 +1967,14 @@ void DAGTypeLegalizer::ExpandFloatRes_FNEG(SDNode *N, SDValue &Lo,
  Hi = DAG.getNode(ISD::FNEG, dl, Hi.getValueType(), Hi);
 }

+void DAGTypeLegalizer::ExpandFloatRes_AssertNoFPClass(SDNode *N, SDValue &Lo,


Possible that this mainly just applies to the ppcf128 type?

arsenm · 2025-08-07T00:01:14Z

llvm/lib/CodeGen/SelectionDAG/LegalizeFloatTypes.cpp

+  Lo = DAG.getNode(ISD::AssertNoFPClass, dl, Lo.getValueType(), Lo);
+  Hi = DAG.getNode(ISD::AssertNoFPClass, dl, Hi.getValueType(), Hi);


I'm not sure it's safe to directly take the value for both halves, and if it is it's only in double double cases like ppcf128. For ppcf128, it might not even apply to both halves. The safest option to get the compilation to stop failing is to drop the operation altogether and revisit the details of when it's preservable later

Looking at PPCTargetLowering::LowerIS_FPCLASS, it appears that for ppcf128 the class is determined by element 1. So I think propagating the info for element 1 is correct. I don't have any evidence about element 0, so I think it should not be propagated there. I am okay with not propagating the assert for both elements for now. I don't see any type tests or asserts so it's hard to be confident that only ppcf128 reaches here, so I think ppcf128 specific stuff like element 1 is class should be if protected and there be a general clear both path.

Ah, yeah, I noticed I have a typo here.

As Roland mentioned, the class comes from the first element, which is what I meant:

SDValue PPCTargetLowering::LowerIS_FPCLASS(SDValue Op, SelectionDAG &DAG) const { assert(Subtarget.hasP9Vector() && "Test data class requires Power9"); SDValue LHS = Op.getOperand(0); uint64_t RHSC = Op.getConstantOperandVal(1); SDLoc Dl(Op); FPClassTest Category = static_cast<FPClassTest>(RHSC); if (LHS.getValueType() == MVT::ppcf128) { // The higher part determines the value class. LHS = DAG.getNode(ISD::EXTRACT_ELEMENT, Dl, MVT::f64, LHS, DAG.getConstant(1, Dl, MVT::i32)); } return getDataClassTest(LHS, Category, Dl, DAG, Subtarget); }

Unless I am misunderstanding, based on the two comments, propagating the information from element 1 is correct but it sounds like should just drop the operation for now.

@arsenm Dumb question, sorry - but what exactly does dropping the operation look like in this case? I assume the Hi and Lo would not be set then?

arsenm · 2025-08-07T00:01:18Z

llvm/test/CodeGen/PowerPC/nofpclass.ll

+; RUN: llc -verify-machineinstrs  -mtriple=powerpc64le-unknown-linux-gnu < %s \
+; RUN:   | FileCheck %s
+; RUN: llc -verify-machineinstrs  -mtriple=powerpc64-ibm-aix-xcoff < %s \
+; RUN:   | FileCheck %s


Suggested change

; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu < %s \

; RUN: | FileCheck %s

; RUN: llc -verify-machineinstrs -mtriple=powerpc64-ibm-aix-xcoff < %s \

; RUN: | FileCheck %s

; RUN: llc -mtriple=powerpc64le-unknown-linux-gnu < %s | FileCheck %s

; RUN: llc -mtriple=powerpc64-ibm-aix-xcoff < %s | FileCheck %s

arsenm · 2025-08-07T00:02:28Z

llvm/test/CodeGen/PowerPC/nofpclass.ll

+; CHECK-NEXT:    blr
+entry:
+  ret ppc_fp128 %s
+}


Ideally would test with more masks, and some cases that demonstrate downstream effects of preserving it after legalization. That's best left to a follow up that tries to preserve it though

amy-kwan requested review from arsenm, wzssyqa, mandlebug and RolandF77 August 6, 2025 18:17

amy-kwan self-assigned this Aug 6, 2025

llvmbot added backend:PowerPC llvm:SelectionDAG SelectionDAGISel as well labels Aug 6, 2025

amy-kwan mentioned this pull request Aug 6, 2025

LLVM 21.1.0: Unable to expand result for ppcf128 with ISD::AssertNoFPClass #151375

Open

amy-kwan mentioned this pull request Aug 6, 2025

CodeGen: Add ISD::AssertNoFPClass #138839

Merged

amy-kwan commented Aug 6, 2025

View reviewed changes

arsenm reviewed Aug 7, 2025

View reviewed changes

arsenm requested a review from jcranmer-intel August 7, 2025 00:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[PowerPC][CodeGen] Expand ISD::AssertNoFPClass for ppc_fp128 #152357

[PowerPC][CodeGen] Expand ISD::AssertNoFPClass for ppc_fp128 #152357

amy-kwan commented Aug 6, 2025

Uh oh!

llvmbot commented Aug 6, 2025

Uh oh!

llvmbot commented Aug 6, 2025

Uh oh!

amy-kwan Aug 6, 2025

Uh oh!

arsenm Aug 7, 2025

Uh oh!

RolandF77 Aug 7, 2025

Uh oh!

amy-kwan Aug 9, 2025

Uh oh!

arsenm Aug 7, 2025

Uh oh!

arsenm Aug 7, 2025

Uh oh!

Uh oh!

		Lo = DAG.getNode(ISD::AssertNoFPClass, dl, Lo.getValueType(), Lo);
		Hi = DAG.getNode(ISD::AssertNoFPClass, dl, Hi.getValueType(), Hi);

[PowerPC][CodeGen] Expand ISD::AssertNoFPClass for ppc_fp128 #152357

Are you sure you want to change the base?

[PowerPC][CodeGen] Expand ISD::AssertNoFPClass for ppc_fp128 #152357

Conversation

amy-kwan commented Aug 6, 2025

Uh oh!

llvmbot commented Aug 6, 2025

Uh oh!

llvmbot commented Aug 6, 2025

Uh oh!

amy-kwan Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

RolandF77 Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

amy-kwan Aug 9, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

arsenm Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!