Skip to content

[TargetLowering][SelectionDAG] Exploit nneg Flag in UINT_TO_FP #108931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 14, 2024

Conversation

mmarjieh
Copy link
Contributor

@mmarjieh mmarjieh commented Sep 17, 2024

  1. Propagate the nneg flag in WidenVecRes
  2. Use SINT_TO_FP in expandUINT_TO_FP when possible.

@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Sep 17, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 17, 2024

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-llvm-selectiondag

Author: Michael Marjieh (mmarjieh)

Changes
  1. Propogate the nneg flag in WidenVecRes
  2. Use SINT_TO_FP in expandUINT_TO_FP when possible.

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

3 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp (+5-5)
  • (modified) llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp (+14-6)
  • (modified) llvm/test/CodeGen/VE/Scalar/cast.ll (+10)
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
index 482f88e5c86de7..249cda171fbbf0 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
@@ -5205,7 +5205,7 @@ SDValue DAGTypeLegalizer::WidenVecRes_Convert(SDNode *N) {
   if (N->getOpcode() == ISD::ZERO_EXTEND &&
       getTypeAction(InVT) == TargetLowering::TypePromoteInteger &&
       TLI.getTypeToTransformTo(Ctx, InVT).getScalarSizeInBits() !=
-      WidenVT.getScalarSizeInBits()) {
+          WidenVT.getScalarSizeInBits()) {
     InOp = ZExtPromotedInteger(InOp);
     InVT = InOp.getValueType();
     if (WidenVT.getScalarSizeInBits() < InVT.getScalarSizeInBits())
@@ -5222,7 +5222,7 @@ SDValue DAGTypeLegalizer::WidenVecRes_Convert(SDNode *N) {
     InVTEC = InVT.getVectorElementCount();
     if (InVTEC == WidenEC) {
       if (N->getNumOperands() == 1)
-        return DAG.getNode(Opcode, DL, WidenVT, InOp);
+        return DAG.getNode(Opcode, DL, WidenVT, InOp, Flags);
       if (N->getNumOperands() == 3) {
         assert(N->isVPOpcode() && "Expected VP opcode");
         SDValue Mask =
@@ -5258,7 +5258,7 @@ SDValue DAGTypeLegalizer::WidenVecRes_Convert(SDNode *N) {
       Ops[0] = InOp;
       SDValue InVec = DAG.getNode(ISD::CONCAT_VECTORS, DL, InWidenVT, Ops);
       if (N->getNumOperands() == 1)
-        return DAG.getNode(Opcode, DL, WidenVT, InVec);
+        return DAG.getNode(Opcode, DL, WidenVT, InVec, Flags);
       return DAG.getNode(Opcode, DL, WidenVT, InVec, N->getOperand(1), Flags);
     }
 
@@ -5267,7 +5267,7 @@ SDValue DAGTypeLegalizer::WidenVecRes_Convert(SDNode *N) {
                                   DAG.getVectorIdxConstant(0, DL));
       // Extract the input and convert the shorten input vector.
       if (N->getNumOperands() == 1)
-        return DAG.getNode(Opcode, DL, WidenVT, InVal);
+        return DAG.getNode(Opcode, DL, WidenVT, InVal, Flags);
       return DAG.getNode(Opcode, DL, WidenVT, InVal, N->getOperand(1), Flags);
     }
   }
@@ -5282,7 +5282,7 @@ SDValue DAGTypeLegalizer::WidenVecRes_Convert(SDNode *N) {
     SDValue Val = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL, InEltVT, InOp,
                               DAG.getVectorIdxConstant(i, DL));
     if (N->getNumOperands() == 1)
-      Ops[i] = DAG.getNode(Opcode, DL, EltVT, Val);
+      Ops[i] = DAG.getNode(Opcode, DL, EltVT, Val, Flags);
     else
       Ops[i] = DAG.getNode(Opcode, DL, EltVT, Val, N->getOperand(1), Flags);
   }
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index 95937886280685..0522958d5221c1 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -8361,18 +8361,26 @@ bool TargetLowering::expandFP_TO_UINT(SDNode *Node, SDValue &Result,
 }
 
 bool TargetLowering::expandUINT_TO_FP(SDNode *Node, SDValue &Result,
-                                      SDValue &Chain,
-                                      SelectionDAG &DAG) const {
+                                      SDValue &Chain, SelectionDAG &DAG) const {
+  SDValue Src = Node->getOperand(0);
+  EVT SrcVT = Src.getValueType();
+  EVT DstVT = Node->getValueType(0);
+
+  // If the input is known to be non-negative and SINT_TO_FP is legal then use
+  // it.
+  if (Node->getFlags().hasNonNeg() &&
+      isOperationLegalOrCustom(ISD::SINT_TO_FP, DstVT)) {
+    Result =
+        DAG.getNode(ISD::SINT_TO_FP, SDLoc(Node), DstVT, Node->getOperand(0));
+    return true;
+  }
+
   // This transform is not correct for converting 0 when rounding mode is set
   // to round toward negative infinity which will produce -0.0. So disable under
   // strictfp.
   if (Node->isStrictFPOpcode())
     return false;
 
-  SDValue Src = Node->getOperand(0);
-  EVT SrcVT = Src.getValueType();
-  EVT DstVT = Node->getValueType(0);
-
   if (SrcVT.getScalarType() != MVT::i64 || DstVT.getScalarType() != MVT::f64)
     return false;
 
diff --git a/llvm/test/CodeGen/VE/Scalar/cast.ll b/llvm/test/CodeGen/VE/Scalar/cast.ll
index 44782b342f4d0f..9253b5591b351d 100644
--- a/llvm/test/CodeGen/VE/Scalar/cast.ll
+++ b/llvm/test/CodeGen/VE/Scalar/cast.ll
@@ -568,6 +568,16 @@ define float @ull2f(i64 %x) {
   ret float %r
 }
 
+define float @ull2f_nneg(i64 %x) {
+; CHECK-LABEL: ull2f_nneg:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    cvt.d.l %s0, %s0
+; CHECK-NEXT:    cvt.s.d %s0, %s0
+; CHECK-NEXT:    b.l.t (, %s10)
+  %r = uitofp nneg i64 %x to float
+  ret float %r
+}
+
 define double @ull2d(i64 %x) {
 ; CHECK-LABEL: ull2d:
 ; CHECK:       # %bb.0:

// If the input is known to be non-negative and SINT_TO_FP is legal then use
// it.
if (Node->getFlags().hasNonNeg() &&
isOperationLegalOrCustom(ISD::SINT_TO_FP, DstVT)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SINT_TO_FP is one of the opcodes that checks the legality based on the source type, not the result type. This is bad because you really need to know both types

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you suggest?
isOperationLegalOrCustom(ISD::SINT_TO_FP, SrcVT))
?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is what the legalizer actually checks:

Node->getOperand(0).getValueType());

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

%r = uitofp nneg i64 %x to float
ret float %r
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing vector tests? Can you also add strictfp tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find a target that has uitofp marked as Expand and sitofp marked as legal or custom on vector types.
I added a strictfp.

@mmarjieh
Copy link
Contributor Author

@arsenm Can you submit it on my behalf? I don't have write access.

Squashed Commit Message:
[TargetLowering][SelectionDAG] Exploit nneg Flag in UINT_TO_FP

Propogate the nneg flag in WidenVecRes
Use SINT_TO_FP in expandUINT_TO_FP when possible.

@arsenm
Copy link
Contributor

arsenm commented Oct 14, 2024

You can just edit the commit description directly?

@mmarjieh
Copy link
Contributor Author

You can just edit the commit description directly?
@arsenm
If I remember correctly, the should be an option to squash the commits, don't you have it near the merge button?

1. Propogate the nneg flag in WidenVecRes
2. Use SINT_TO_FP in expandUINT_TO_FP when possible.
@arsenm
Copy link
Contributor

arsenm commented Oct 14, 2024

You can just edit the commit description directly?
@llvm/pr-subscribers-openmp

If I remember correctly, the should be an option to squash the commits, don't you have it near the merge button?

That is the only option enabled for the llvm repo

@arsenm arsenm merged commit b5600c6 into llvm:main Oct 14, 2024
8 checks passed
@mmarjieh
Copy link
Contributor Author

@arsenm I appreciate your help, thanks!

DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…108931)

1. Propagate the nneg flag in WidenVecRes
2. Use SINT_TO_FP in expandUINT_TO_FP when possible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants