Skip to content
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

[GISEL] G_SPLAT_VECTOR can take a splat that is larger than the vector element #86974

Merged
merged 4 commits into from
Apr 1, 2024

Conversation

michaelmaitland
Copy link
Contributor

This is what SelectionDAG does. We'd like to reuse SelectionDAG patterns.

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 28, 2024

@llvm/pr-subscribers-llvm-globalisel

Author: Michael Maitland (michaelmaitland)

Changes

This is what SelectionDAG does. We'd like to reuse SelectionDAG patterns.


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

3 Files Affected:

  • (modified) llvm/docs/GlobalISel/GenericOpcode.rst (+4)
  • (modified) llvm/lib/CodeGen/MachineVerifier.cpp (+4-3)
  • (modified) llvm/test/MachineVerifier/test_g_splat_vector.mir (+2-2)
diff --git a/llvm/docs/GlobalISel/GenericOpcode.rst b/llvm/docs/GlobalISel/GenericOpcode.rst
index cae2c21b80d7e7..e3ec5272c1e167 100644
--- a/llvm/docs/GlobalISel/GenericOpcode.rst
+++ b/llvm/docs/GlobalISel/GenericOpcode.rst
@@ -690,6 +690,10 @@ G_SPLAT_VECTOR
 
 Create a vector where all elements are the scalar from the source operand.
 
+The type of the operand must be equal to or smaller than the vector element
+type. If the operand is smaller than the vector element type, the scalar is
+implicitly truncated to the vector element type.
+
 Vector Reduction Operations
 ---------------------------
 
diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index e4e05ce9278caf..e568a379b213e6 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -1774,9 +1774,10 @@ void MachineVerifier::verifyPreISelGenericInstruction(const MachineInstr *MI) {
     if (!SrcTy.isScalar())
       report("Source type must be a scalar", MI);
 
-    if (DstTy.getScalarType() != SrcTy)
-      report("Element type of the destination must be the same type as the "
-             "source type",
+    if (TypeSize::isKnownGT(DstTy.getScalarType().getSizeInBits(),
+                            SrcTy.getSizeInBits()))
+      report("Element type of the destination must be the same size or smaller "
+             "than the source type",
              MI);
 
     break;
diff --git a/llvm/test/MachineVerifier/test_g_splat_vector.mir b/llvm/test/MachineVerifier/test_g_splat_vector.mir
index 0d1d8a3e6dcc64..c5d0c1b61f3915 100644
--- a/llvm/test/MachineVerifier/test_g_splat_vector.mir
+++ b/llvm/test/MachineVerifier/test_g_splat_vector.mir
@@ -22,6 +22,6 @@ body:             |
     ; CHECK: Source type must be a scalar
     %6:_(<vscale x 2 x s32>) = G_SPLAT_VECTOR %2
 
-    ; CHECK: Element type of the destination must be the same type as the source type
-    %7:_(<vscale x 2 x s64>) = G_SPLAT_VECTOR %0
+    ; CHECK: Element type of the destination must be the same size or smaller than the source type
+    %7:_(<vscale x 2 x s128>) = G_SPLAT_VECTOR %7
 ...

…or element

This is what SelectionDAG does. We'd like to reuse SelectionDAG patterns.
@@ -690,6 +690,10 @@ G_SPLAT_VECTOR

Create a vector where all elements are the scalar from the source operand.

The type of the operand must be equal to or smaller than the vector element
type. If the operand is smaller than the vector element type, the scalar is
implicitly truncated to the vector element type.
Copy link
Member

Choose a reason for hiding this comment

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

extended?

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 got it all backwards 😆 . The operand should be allowed to be greater than the size of the vector element.

@tschuett
Copy link
Member

The title still says smaller. Assuming that this is again for patterns, but G_SPLAT_VECTOR(G_TRUNC(s128)) should suffice during legalisation?

@tschuett tschuett requested a review from aemerson March 28, 2024 17:16
@michaelmaitland michaelmaitland changed the title [GISEL] G_SPLAT_VECTOR can take a splat that is smaller than the vector element [GISEL] G_SPLAT_VECTOR can take a splat that is larger than the vector element Mar 28, 2024
@michaelmaitland
Copy link
Contributor Author

Assuming that this is again for patterns, but G_SPLAT_VECTOR(G_TRUNC(s128)) should suffice during legalisation?

This is for patterns and for equivalence to SelectionDAG ISD::SPLAT_VECTOR.

@tschuett
Copy link
Member

I am again confused. You want to take i64 splats on 32-bit machines and truncate the i64 to i32? You can just state that i64 are not legal?!?

@michaelmaitland
Copy link
Contributor Author

michaelmaitland commented Mar 28, 2024

I am again confused. You want to take i64 splats on 32-bit machines and truncate the i64 to i32? You can just state that i64 are not legal?!?

From selection dag:

The type of the operand must match the vector element type, except when they are integer types. In this case the operand is allowed to be wider than the vector element type, and is implicitly truncated to it.

I want to do this since the patterns have an sXLen operand which can be s32 on RV32 and I should be able to create a 8b, 16b splat vector. And on RV64 the operand will be sXLen and I want to create 8b, 16b, and 32b splat vectors

Copy link
Contributor

@aemerson aemerson left a comment

Choose a reason for hiding this comment

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

This seems reasonable to me if it allows us to import more patterns.

@michaelmaitland michaelmaitland merged commit da9f06c into llvm:main Apr 1, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants