Skip to content

Conversation

AmrDeveloper
Copy link
Member

Extend the verifier to catch index larger than the size of vector elements in VecShuffleOp

Issue #136487

@llvmbot llvmbot added clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project labels Jun 7, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 7, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clangir

Author: Amr Hesham (AmrDeveloper)

Changes

Extend 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:

  • (modified) clang/lib/CIR/Dialect/IR/CIRDialect.cpp (+8)
  • (added) clang/test/CIR/IR/invalid-vector-shuffle-wrong-index.cir (+16)
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>
+  }
+}

Copy link
Contributor

@xlauko xlauko left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Member

@bcardosolopes bcardosolopes left a comment

Choose a reason for hiding this comment

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

LGTM, one nit


const uint64_t maxValidIndex =
getVec1().getType().getSize() + getVec2().getType().getSize() - 1;
for (const auto &idxAttr : getIndices().getAsRange<cir::IntAttr>()) {
Copy link
Member

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(...);

bcardosolopes pushed a commit to llvm/clangir that referenced this pull request Jun 9, 2025
…#1670)

Backport the VecShuffleOp verifier to catch invalid index

Implemented in llvm/llvm-project#143262
Copy link
Contributor

@andykaylor andykaylor left a comment

Choose a reason for hiding this comment

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

lgtm

// match.
if (getVec1().getType().getElementType() !=
getResult().getType().getElementType()) {
getResult().getType().getElementType())
Copy link
Contributor

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.

@AmrDeveloper AmrDeveloper merged commit f62a8ab into llvm:main Jun 13, 2025
7 checks passed
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
Extend the verifier to catch index larger than the size of vector
elements in VecShuffleOp

Issue llvm#136487
akuhlens pushed a commit to akuhlens/llvm-project that referenced this pull request Jun 24, 2025
Extend the verifier to catch index larger than the size of vector
elements in VecShuffleOp

Issue llvm#136487
lanza pushed a commit to lanza/llvm-project that referenced this pull request Aug 11, 2025
…llvm#1670)

Backport the VecShuffleOp verifier to catch invalid index

Implemented in llvm#143262
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category ClangIR Anything related to the ClangIR project

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants