add narrowExtractedVectorUnaryOp to simplify cast nodes

[vedantparanjape-amd]

No description provided.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff d0dcf06ab8723cc4358ad446354cce875dd89577 e263e5f0c99aaf8d9451228518e376b24b5d7f57 -- llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 23dd91e07f..3a01dfa334 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -24086,11 +24086,11 @@ static SDValue narrowInsertExtractVectorBinOp(SDNode *Extract,
 /// If we are extracting a subvector produced by a wide unary operator try
 /// to use a narrow unary operator and/or avoid extraction.
 static SDValue narrowExtractedVectorUnaryOp(SDNode *Extract, SelectionDAG &DAG,
-                                          bool LegalOperations) {
+                                            bool LegalOperations) {
   const TargetLowering &TLI = DAG.getTargetLoweringInfo();
   SDValue UnaryOp = Extract->getOperand(0);
   unsigned UnaryOpcode = UnaryOp.getOpcode();
-  
+
   if (UnaryOpcode != ISD::FP_TO_SINT || UnaryOp->getNumValues() != 1)
     return SDValue();
 
@@ -24102,7 +24102,7 @@ static SDValue narrowExtractedVectorUnaryOp(SDNode *Extract, SelectionDAG &DAG,
   EVT WideUVT = UnaryOp.getValueType();
   if (!WideUVT.isFixedLengthVector())
     return SDValue();
-  
+
   EVT VT = Extract->getValueType(0);
   unsigned ExtractIndex = ExtractIndexC->getZExtValue();
   assert(ExtractIndex % VT.getVectorNumElements() == 0 &&
@@ -24124,9 +24124,11 @@ static SDValue narrowExtractedVectorUnaryOp(SDNode *Extract, SelectionDAG &DAG,
                                              LegalOperations))
     return SDValue();
 
-  EVT NarrowUEltVT = EVT::getVectorVT(*DAG.getContext(), UnaryOp.getOperand(0).getValueType().getScalarType(),
-                                   WideNumElts / NarrowingRatio);
-  // if (!TLI.isOperationLegalOrCustomOrPromote(ISD::EXTRACT_SUBVECTOR, NarrowUEltVT,
+  EVT NarrowUEltVT = EVT::getVectorVT(
+      *DAG.getContext(), UnaryOp.getOperand(0).getValueType().getScalarType(),
+      WideNumElts / NarrowingRatio);
+  // if (!TLI.isOperationLegalOrCustomOrPromote(ISD::EXTRACT_SUBVECTOR,
+  // NarrowUEltVT,
   //                                            LegalOperations))
   //   return SDValue();
 

[RKSimon]

You need to add similar code to extract_subvector stage here: https://github.com/llvm/llvm-project/blob/170c525d79a4ab3659041b0655ac9697768fc915/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp#L24157-L24169

[vedantparanjape-amd]

Ah, I see. so there should a extract_subvec before fp_to_sint as well. I thought such a pattern would be already handled.

[RKSimon]

What you're doing is fold extract_subvector(fp_to_sint(x),c) -> fp_to_sint(extract_subvector(x,c))

[vedantparanjape-amd]

I made the change (updated in the PR), it now fails at isel. The transformation seems okay to me:

Optimized type-legalized selection DAG: %bb.0 'fptosi_4f16_to_4i32:'
SelectionDAG has 8 nodes:
  t0: ch,glue = EntryToken
        t2: v8f16,ch = CopyFromReg t0, Register:v8f16 %1
      t10: v4f16 = extract_subvector t2, Constant:i64<0>
    t11: v4i32 = fp_to_sint t10
  t7: ch = CopyToReg t0, Register:v4i32 %2, t11

ISEL: Starting selection on root node: t16: v4f32 = X86ISD::VFPEXT t2
ISEL: Starting pattern match
  Initial Opcode index to 626311
  Match failed at index 626314
  Continuing at 626400
  Skipped scope entry (due to false predicate) at index 626404, continuing at 626435
  TypeSwitch[v4f32] from 626438 to 626441
  Match failed at index 626441
  Continuing at 626467
  Continuing at 626468
LLVM ERROR: Cannot select: t16: v4f32 = X86ISD::VFPEXT t2
  t2: v8f16,ch = CopyFromReg t0, Register:v8f16 %1
    t1: v8f16 = Register %1
In function: fptosi_4f16_to_4i32
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:

[RKSimon]

Add return SDValue() here - if the above if() failed

[vedantparanjape-amd]

Yes, did this. But it doesn't help with the crash.

[RKSimon]

Yes, you're right - we're getting stopped by legality tests as we have poor src/dst legality tests for cast ops.

[vedantparanjape-amd]

Yes, you're right - we're getting stopped by legality tests as we have poor src/dst legality tests for cast ops.

I see, is it something that can be worked on ?

[RKSimon]

Yes, you're right - we're getting stopped by legality tests as we have poor src/dst legality tests for cast ops.

I see, is it something that can be worked on ?

It isn't straightforward - cast ops require legality checks on the src/dst type pairs which isn't stored.