Skip to content

[LegalizeVectorTypes] Allow non-undef vectors when widening insert_subvector op #115110

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

Closed

Conversation

lukel97
Copy link
Contributor

@lukel97 lukel97 commented Nov 6, 2024

When widening an insert_subvector operand there's currently a restriction that the vector being inserted into has to be undef.

It looks like this is just a conservative restriction from when it was first added: https://reviews.llvm.org/D102501

Nothing comes to mind as to why we can't allow non-undef vectors.

Fixes #114900

…bvector op

When widening an insert_subvector operand there's currently a restriction that the vector being inserted into has to be undef.

It looks like this is just a conservative restriction from when it was first added: https://reviews.llvm.org/D102501

Nothing comes to mind as to why we can't allow non-undef vectors.

Fixes llvm#114900
@llvmbot llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Nov 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 6, 2024

@llvm/pr-subscribers-llvm-selectiondag

Author: Luke Lau (lukel97)

Changes

When widening an insert_subvector operand there's currently a restriction that the vector being inserted into has to be undef.

It looks like this is just a conservative restriction from when it was first added: https://reviews.llvm.org/D102501

Nothing comes to mind as to why we can't allow non-undef vectors.

Fixes #114900


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp (+1-1)
  • (modified) llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert-subvector.ll (+9)
diff --git a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
index eccda73548e874..64110aafb16126 100644
--- a/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
@@ -7066,7 +7066,7 @@ SDValue DAGTypeLegalizer::WidenVecOp_INSERT_SUBVECTOR(SDNode *N) {
 
   // We need to make sure that the indices are still valid, otherwise we might
   // widen what was previously well-defined to something undefined.
-  if (IndicesValid && InVec.isUndef() && N->getConstantOperandVal(2) == 0)
+  if (IndicesValid && N->getConstantOperandVal(2) == 0)
     return DAG.getNode(ISD::INSERT_SUBVECTOR, SDLoc(N), VT, InVec, SubVec,
                        N->getOperand(2));
 
diff --git a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert-subvector.ll b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert-subvector.ll
index 5581754b0721a5..8b4614a08b6583 100644
--- a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert-subvector.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert-subvector.ll
@@ -1000,3 +1000,12 @@ define <4 x i32> @insert_extract_v8i32_v2i32_0(<2 x i32> %v) {
   %2 = call <4 x i32> @llvm.vector.extract.v4i32.v8i32(<8 x i32> %1, i64 0)
   ret <4 x i32> %2
 }
+
+define <vscale x 24 x i8> @insert_nxv24i8_v48i8_0(<vscale x 24 x i8> %v, <48 x i8> %w) vscale_range(2) {
+; CHECK-LABEL: insert_nxv24i8_v48i8_0:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vmv4r.v v8, v12
+; CHECK-NEXT:    ret
+  %x = call <vscale x 24 x i8> @llvm.vector.insert.v48i8.nxv24i8(<vscale x 24 x i8> %v, <48 x i8> %w, i64 0)
+  ret <vscale x 24 x i8> %x
+}

@topperc
Copy link
Collaborator

topperc commented Nov 6, 2024

Isn't the test case in #114900 inserting into an undef vector?

@lukel97
Copy link
Contributor Author

lukel97 commented Nov 6, 2024

Isn't the test case in #114900 inserting into an undef vector?

Argh you're right. Not sure where I added in the vector, it compiles fine once the vscale_range attribute is fixed. Closing this since I guess we don't need it

@lukel97 lukel97 closed this Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:SelectionDAG SelectionDAGISel as well
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't know how to widen the operands for INSERT_SUBVECTOR during RISCV codegen
3 participants