Skip to content

[SystemZ] Use the EVT version of getVectorVT() in combineTruncateExtract(). #100150

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 1 commit into from
Jul 26, 2024

Conversation

JonPsson1
Copy link
Contributor

A test case showed up where the new vector type is v24i16, which is not a simple
MVT. In order to get an extended value type for cases like this, EVT::getVectorVT()
needs to be called instead of MVT::getVectorVT(), otherwise the following call
to getVectorElementType() in combineExtract() will fail.

(This showed up in Csmith testing and looks to have been a potential problem for at least 1.5 years.)

There are other places where MVT::getVectorVT() is called, but hopefully they are ok.

@llvmbot
Copy link
Member

llvmbot commented Jul 23, 2024

@llvm/pr-subscribers-backend-systemz

Author: Jonas Paulsson (JonPsson1)

Changes

A test case showed up where the new vector type is v24i16, which is not a simple
MVT. In order to get an extended value type for cases like this, EVT::getVectorVT()
needs to be called instead of MVT::getVectorVT(), otherwise the following call
to getVectorElementType() in combineExtract() will fail.

(This showed up in Csmith testing and looks to have been a potential problem for at least 1.5 years.)

There are other places where MVT::getVectorVT() is called, but hopefully they are ok.


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

2 Files Affected:

  • (modified) llvm/lib/Target/SystemZ/SystemZISelLowering.cpp (+2-1)
  • (modified) llvm/test/CodeGen/SystemZ/vec-combine-01.ll (+10)
diff --git a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
index b2b88143354a5..383393914a169 100644
--- a/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp
@@ -6653,7 +6653,8 @@ SDValue SystemZTargetLowering::combineTruncateExtract(
 
           // Defer the creation of the bitcast from X to combineExtract,
           // which might be able to optimize the extraction.
-          VecVT = MVT::getVectorVT(MVT::getIntegerVT(TruncBytes * 8),
+          VecVT = EVT::getVectorVT(*DCI.DAG.getContext(),
+                                   MVT::getIntegerVT(TruncBytes * 8),
                                    VecVT.getStoreSize() / TruncBytes);
           EVT ResVT = (TruncBytes < 4 ? MVT::i32 : TruncVT);
           return combineExtract(DL, ResVT, VecVT, Vec, NewIndex, DCI, true);
diff --git a/llvm/test/CodeGen/SystemZ/vec-combine-01.ll b/llvm/test/CodeGen/SystemZ/vec-combine-01.ll
index 6f0abd6ea5baf..16231b2d89526 100644
--- a/llvm/test/CodeGen/SystemZ/vec-combine-01.ll
+++ b/llvm/test/CodeGen/SystemZ/vec-combine-01.ll
@@ -153,3 +153,13 @@ define void @f7(ptr %ptr1, ptr %ptr2, ptr %ptr3, ptr %ptr4) {
   store i8 %trunc3, ptr %ptr4
   ret void
 }
+
+; Test that a truncating store with a non-simple VT can be handled.
+define void @f8(ptr %src, ptr %dst) {
+; CHECK-LABEL: f8:
+  %1 = load <12 x i32>, ptr %src, align 64
+  %2 = extractelement <12 x i32> %1, i64 11
+  %3 = trunc i32 %2 to i16
+  store i16 %3, ptr %dst, align 2
+  ret void
+}

Copy link
Member

@uweigand uweigand left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@JonPsson1 JonPsson1 merged commit 22bc9db into llvm:main Jul 26, 2024
9 checks passed
@JonPsson1 JonPsson1 deleted the VecVTDAGComb branch July 26, 2024 12:33
@tstellar tstellar added this to the LLVM 19.X Release milestone Nov 16, 2024
@tstellar
Copy link
Collaborator

/cherry-pick 22bc9db

@llvmbot
Copy link
Member

llvmbot commented Nov 16, 2024

Failed to create pull request for issue100150 https://github.com/llvm/llvm-project/actions/runs/11873454462

tru pushed a commit to llvmbot/llvm-project that referenced this pull request Nov 18, 2024
…act(). (llvm#100150)

A test case showed up where the new vector type is v24i16, which is not a simple
MVT. In order to get an extended value type for cases like this, EVT::getVectorVT()
needs to be called instead of MVT::getVectorVT(), otherwise the following call
to getVectorElementType() in combineExtract() will fail.

(cherry picked from commit 22bc9db)
nikic pushed a commit to rust-lang/llvm-project that referenced this pull request Nov 20, 2024
…act(). (llvm#100150)

A test case showed up where the new vector type is v24i16, which is not a simple
MVT. In order to get an extended value type for cases like this, EVT::getVectorVT()
needs to be called instead of MVT::getVectorVT(), otherwise the following call
to getVectorElementType() in combineExtract() will fail.

(cherry picked from commit 22bc9db)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants