[Mlir-commits] [mlir] [mlir][VectorToLLVM] Fix crash in VectorInsertOpConversion with dynamic index (PR #183783)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Mehdi Amini (joker-eph)

<details>
<summary>Changes</summary>

VectorInsertOpConversion crashes with an assertion failure when inserting a sub-vector at a dynamic position into a multi-dimensional vector. The pattern calls getAsIntegers() on the position, which asserts that all fold results are compile-time constant attributes.

The existing guard (checking llvm::IsaPred<Attribute>) only covered the case where a scalar is inserted into the innermost dimension (the extractvalue path). The guard was missing for the insertvalue path when inserting a sub-vector at a dynamic position into a nested aggregate.

Fix: add the same guard before the llvm.insertvalue creation to return failure() gracefully when any position index is dynamic, matching the behavior of VectorExtractOpConversion.

Fixes #<!-- -->177829

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


2 Files Affected:

- (modified) mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp (+5) 
- (modified) mlir/test/Conversion/VectorToLLVM/vector-to-llvm-interface.mlir (+14) 


``````````diff
diff --git a/mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp b/mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp
index 05d541fe80356..9d81702581131 100644
--- a/mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp
+++ b/mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp
@@ -1279,6 +1279,11 @@ class VectorInsertOpConversion
 
     Value result = sourceAggregate;
     if (isNestedAggregate) {
+      if (!llvm::all_of(positionOf1DVectorWithinAggregate,
+                        llvm::IsaPred<Attribute>)) {
+        // llvm.insertvalue does not support dynamic dimensions.
+        return failure();
+      }
       result = LLVM::InsertValueOp::create(
           rewriter, loc, adaptor.getDest(), sourceAggregate,
           getAsIntegers(positionOf1DVectorWithinAggregate));
diff --git a/mlir/test/Conversion/VectorToLLVM/vector-to-llvm-interface.mlir b/mlir/test/Conversion/VectorToLLVM/vector-to-llvm-interface.mlir
index cb48ca3374e8d..49c55f5b54496 100644
--- a/mlir/test/Conversion/VectorToLLVM/vector-to-llvm-interface.mlir
+++ b/mlir/test/Conversion/VectorToLLVM/vector-to-llvm-interface.mlir
@@ -726,6 +726,20 @@ func.func @insert_scalar_into_vec_2d_f32_dynamic_idx_fail(%arg0: vector<2x16xf32
 
 // -----
 
+// Regression test for https://github.com/llvm/llvm-project/issues/177829:
+// Inserting a 1D sub-vector at a dynamic position into a 2D vector must not
+// crash. llvm.insertvalue does not support dynamic dimensions, so the pattern
+// must bail out gracefully.
+func.func @insert_vec_1d_into_vec_2d_dynamic_idx_fail(%arg0: vector<4xi1>, %arg1: vector<2x4xi1>, %idx: index) -> vector<2x4xi1> {
+  %0 = vector.insert %arg0, %arg1[%idx] : vector<4xi1> into vector<2x4xi1>
+  return %0 : vector<2x4xi1>
+}
+
+// CHECK-LABEL: @insert_vec_1d_into_vec_2d_dynamic_idx_fail
+//       CHECK:   vector.insert
+
+// -----
+
 func.func @insert_scalar_from_vec_2d_f32_dynamic_idxs_compile_time_const(%arg : vector<4x1xf32>) -> vector<4x1xf32> {
   %0 = arith.constant 0 : index
   %1 = arith.constant 1.0 : f32

``````````

</details>


https://github.com/llvm/llvm-project/pull/183783