-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[CIR] Extend VecShuffleOp verifier to catch invalid index #143262
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
[CIR] Extend VecShuffleOp verifier to catch invalid index #143262
Conversation
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: Amr Hesham (AmrDeveloper) ChangesExtend the verifier to catch index larger than the size of vector elements in VecShuffleOp Issue #136487 Full diff: https://github.com/llvm/llvm-project/pull/143262.diff 2 Files Affected:
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index bfd3a0a62a8e7..b0678ee737a62 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -1599,6 +1599,14 @@ LogicalResult cir::VecShuffleOp::verify() {
<< " and " << getResult().getType() << " don't match";
}
+ const uint64_t maxValidIndex =
+ getVec1().getType().getSize() + getVec2().getType().getSize() - 1;
+ for (const auto &idxAttr : getIndices().getAsRange<cir::IntAttr>()) {
+ if (idxAttr.getUInt() > maxValidIndex)
+ return emitOpError() << ": index for __builtin_shufflevector must be "
+ "less than the total number of vector elements";
+ }
+
return success();
}
diff --git a/clang/test/CIR/IR/invalid-vector-shuffle-wrong-index.cir b/clang/test/CIR/IR/invalid-vector-shuffle-wrong-index.cir
new file mode 100644
index 0000000000000..375b2d3dc563e
--- /dev/null
+++ b/clang/test/CIR/IR/invalid-vector-shuffle-wrong-index.cir
@@ -0,0 +1,16 @@
+// RUN: cir-opt %s -verify-diagnostics -split-input-file
+
+!s32i = !cir.int<s, 32>
+!s64i = !cir.int<s, 64>
+
+module {
+ cir.func @fold_shuffle_vector_op_test() -> !cir.vector<4 x !s32i> {
+ %vec_1 = cir.const #cir.const_vector<[#cir.int<1> : !s32i, #cir.int<3> : !s32i, #cir.int<5> : !s32i, #cir.int<7> : !s32i]> : !cir.vector<4 x !s32i>
+ %vec_2 = cir.const #cir.const_vector<[#cir.int<2> : !s32i, #cir.int<4> : !s32i, #cir.int<6> : !s32i, #cir.int<8> : !s32i]> : !cir.vector<4 x !s32i>
+
+ // expected-error @below {{index for __builtin_shufflevector must be less than the total number of vector elements}}
+ %new_vec = cir.vec.shuffle(%vec_1, %vec_2 : !cir.vector<4 x !s32i>) [#cir.int<9> : !s64i, #cir.int<4> : !s64i,
+ #cir.int<1> : !s64i, #cir.int<5> : !s64i] : !cir.vector<4 x !s32i>
+ cir.return %new_vec : !cir.vector<4 x !s32i>
+ }
+}
|
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
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, one nit
@@ -1599,6 +1599,14 @@ LogicalResult cir::VecShuffleOp::verify() { | |||
<< " and " << getResult().getType() << " don't match"; | |||
} | |||
|
|||
const uint64_t maxValidIndex = | |||
getVec1().getType().getSize() + getVec2().getType().getSize() - 1; | |||
for (const auto &idxAttr : getIndices().getAsRange<cir::IntAttr>()) { |
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.
Maybe replace the for
here by
if (llvm::any_of(getIndices().getAsRange<cir::IntAttr>(),
[&](cir::IntAttr idxAttr) {
return idxAttr.getSInt() != -1 && idxAttr.getUInt() > maxValidIndex;
}) emitError(...);
…#1670) Backport the VecShuffleOp verifier to catch invalid index Implemented in llvm/llvm-project#143262
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
|
||
// The element types of the two input vectors and of the result type must | ||
// match. | ||
if (getVec1().getType().getElementType() != | ||
getResult().getType().getElementType()) { | ||
getResult().getType().getElementType()) |
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.
I might have left the braces here since both the condition and the true-expression span multiple lines, but it's really fine either way.
Extend the verifier to catch index larger than the size of vector elements in VecShuffleOp Issue llvm#136487
Extend the verifier to catch index larger than the size of vector elements in VecShuffleOp Issue llvm#136487
Extend the verifier to catch index larger than the size of vector elements in VecShuffleOp
Issue #136487