-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[ConstantFolding] Fold scalable shufflevector of poison/undef. #143475
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
Conversation
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-llvm-transforms Author: Craig Topper (topperc) ChangesFull diff: https://github.com/llvm/llvm-project/pull/143475.diff 2 Files Affected:
diff --git a/llvm/lib/IR/ConstantFold.cpp b/llvm/lib/IR/ConstantFold.cpp
index b9db402fe9562..6c8850aae0476 100644
--- a/llvm/lib/IR/ConstantFold.cpp
+++ b/llvm/lib/IR/ConstantFold.cpp
@@ -463,10 +463,7 @@ Constant *llvm::ConstantFoldShuffleVectorInstruction(Constant *V1, Constant *V2,
Constant *Elt =
ConstantExpr::getExtractElement(V1, ConstantInt::get(Ty, 0));
- if (Elt->isNullValue()) {
- auto *VTy = VectorType::get(EltTy, MaskEltCount);
- return ConstantAggregateZero::get(VTy);
- } else if (!MaskEltCount.isScalable())
+ if (!MaskEltCount.isScalable() || Elt->isNullValue() || isa<UndefValue>(Elt))
return ConstantVector::getSplat(MaskEltCount, Elt);
}
diff --git a/llvm/test/Transforms/InstSimplify/shufflevector.ll b/llvm/test/Transforms/InstSimplify/shufflevector.ll
index 0442bd721974b..feab024a29213 100644
--- a/llvm/test/Transforms/InstSimplify/shufflevector.ll
+++ b/llvm/test/Transforms/InstSimplify/shufflevector.ll
@@ -337,3 +337,19 @@ define <4 x i32> @not_fold_identity2(<4 x i32> %x) {
%revshuf = shufflevector <4 x i32> %shuf, <4 x i32> poison, <4 x i32> <i32 3, i32 2, i32 1, i32 0>
ret <4 x i32> %revshuf
}
+
+define <vscale x 2 x i32> @scalable_splat_undef() {
+; CHECK-LABEL: @scalable_splat_undef(
+; CHECK-NEXT: ret <vscale x 2 x i32> undef
+;
+ %shuf = shufflevector <vscale x 2 x i32> undef, <vscale x 2 x i32> undef, <vscale x 2 x i32> zeroinitializer
+ ret <vscale x 2 x i32> %shuf
+}
+
+define <vscale x 2 x i32> @scalable_splat_poison() {
+; CHECK-LABEL: @scalable_splat_poison(
+; CHECK-NEXT: ret <vscale x 2 x i32> poison
+;
+ %shuf = shufflevector <vscale x 2 x i32> poison, <vscale x 2 x i32> poison, <vscale x 2 x i32> zeroinitializer
+ ret <vscale x 2 x i32> %shuf
+}
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions cpp -- llvm/lib/IR/ConstantFold.cpp View the diff from clang-format here.diff --git a/llvm/lib/IR/ConstantFold.cpp b/llvm/lib/IR/ConstantFold.cpp
index d4ad21e69..9e8ae34a9 100644
--- a/llvm/lib/IR/ConstantFold.cpp
+++ b/llvm/lib/IR/ConstantFold.cpp
@@ -465,7 +465,8 @@ Constant *llvm::ConstantFoldShuffleVectorInstruction(Constant *V1, Constant *V2,
// For scalable vectors, make sure this doesn't fold back into a
// shufflevector.
- if (!MaskEltCount.isScalable() || Elt->isNullValue() || isa<UndefValue>(Elt))
+ if (!MaskEltCount.isScalable() || Elt->isNullValue() ||
+ isa<UndefValue>(Elt))
return ConstantVector::getSplat(MaskEltCount, Elt);
}
|
You can test this locally with the following command:git diff -U0 --pickaxe-regex -S '([^a-zA-Z0-9#_-]undef[^a-zA-Z0-9_-]|UndefValue::get)' 'HEAD~1' HEAD llvm/lib/IR/ConstantFold.cpp llvm/test/Transforms/InstSimplify/shufflevector.ll The following files introduce new uses of undef:
Undef is now deprecated and should only be used in the rare cases where no replacement is possible. For example, a load of uninitialized memory yields In tests, avoid using For example, this is considered a bad practice: define void @fn() {
...
br i1 undef, ...
} Please use the following instead: define void @fn(i1 %cond) {
...
br i1 %cond, ...
} Please refer to the Undefined Behavior Manual for more information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
auto *VTy = VectorType::get(EltTy, MaskEltCount); | ||
return ConstantAggregateZero::get(VTy); | ||
} else if (!MaskEltCount.isScalable()) | ||
if (!MaskEltCount.isScalable() || Elt->isNullValue() || isa<UndefValue>(Elt)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It took me a while to get why we need this condition. I'd add a comment like
// For scalable vectors, make sure this doesn't fold back into a shufflevector.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/92/builds/20338 Here is the relevant piece of the build log for the reference
|
No description provided.