Skip to content

Conversation

banach-space
Copy link
Contributor

This patch updates the following ops to use result (instead of res)
as the name for their result argument:

  • vector.scalable.insert
  • vector.scalable.extract
  • vector.insert_strided_slice

This change ensures naming consistency with other ops in the vector
dialect. It addresses part of:

This patch updates the following ops to use `result` (instead of `res`)
as the name for their result argument:
  * `vector.scalable.insert`
  * `vector.scalable.extract`
  * `vector.insert_strided_slice`

This change ensures naming consistency with other ops in the `vector`
dialect. It addresses part of:
* llvm#131602
@llvmbot
Copy link
Member

llvmbot commented Jun 18, 2025

@llvm/pr-subscribers-mlir-vector

@llvm/pr-subscribers-mlir

Author: Andrzej Warzyński (banach-space)

Changes

This patch updates the following ops to use result (instead of res)
as the name for their result argument:

  • vector.scalable.insert
  • vector.scalable.extract
  • vector.insert_strided_slice

This change ensures naming consistency with other ops in the vector
dialect. It addresses part of:


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

1 Files Affected:

  • (modified) mlir/include/mlir/Dialect/Vector/IR/VectorOps.td (+8-8)
diff --git a/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td b/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td
index 8353314ed958b..5373d5e356752 100644
--- a/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td
+++ b/mlir/include/mlir/Dialect/Vector/IR/VectorOps.td
@@ -959,7 +959,7 @@ def Vector_InsertOp :
 def Vector_ScalableInsertOp :
   Vector_Op<"scalable.insert", [Pure,
        AllElementTypesMatch<["valueToStore", "dest"]>,
-       AllTypesMatch<["dest", "res"]>,
+       AllTypesMatch<["dest", "result"]>,
        PredOpTrait<"position is a multiple of the source length.",
         CPred<
           "(getPos() % getSourceVectorType().getNumElements()) == 0"
@@ -967,7 +967,7 @@ def Vector_ScalableInsertOp :
      Arguments<(ins VectorOfRank<[1]>:$valueToStore,
                     ScalableVectorOfRank<[1]>:$dest,
                     I64Attr:$pos)>,
-     Results<(outs ScalableVectorOfRank<[1]>:$res)> {
+     Results<(outs ScalableVectorOfRank<[1]>:$result)> {
   let summary = "insert subvector into scalable vector operation";
   // NOTE: This operation is designed to map to `llvm.vector.insert`, and its
   //       documentation should be kept aligned with LLVM IR:
@@ -1015,14 +1015,14 @@ def Vector_ScalableInsertOp :
 
 def Vector_ScalableExtractOp :
   Vector_Op<"scalable.extract", [Pure,
-       AllElementTypesMatch<["source", "res"]>,
+       AllElementTypesMatch<["source", "result"]>,
        PredOpTrait<"position is a multiple of the result length.",
         CPred<
           "(getPos() % getResultVectorType().getNumElements()) == 0"
         >>]>,
      Arguments<(ins ScalableVectorOfRank<[1]>:$source,
                     I64Attr:$pos)>,
-     Results<(outs VectorOfRank<[1]>:$res)> {
+     Results<(outs VectorOfRank<[1]>:$result)> {
   let summary = "extract subvector from scalable vector operation";
   // NOTE: This operation is designed to map to `llvm.vector.extract`, and its
   //       documentation should be kept aligned with LLVM IR:
@@ -1051,7 +1051,7 @@ def Vector_ScalableExtractOp :
   }];
 
   let assemblyFormat = [{
-    $source `[` $pos `]` attr-dict `:` type($res) `from` type($source)
+    $source `[` $pos `]` attr-dict `:` type($result) `from` type($source)
   }];
 
   let extraClassDeclaration = extraPoisonClassDeclaration # [{
@@ -1059,7 +1059,7 @@ def Vector_ScalableExtractOp :
       return ::llvm::cast<VectorType>(getSource().getType());
     }
     VectorType getResultVectorType() {
-      return ::llvm::cast<VectorType>(getRes().getType());
+      return ::llvm::cast<VectorType>(getResult().getType());
     }
   }];
 }
@@ -1068,10 +1068,10 @@ def Vector_InsertStridedSliceOp :
   Vector_Op<"insert_strided_slice", [Pure,
     PredOpTrait<"operand #0 and result have same element type",
                  TCresVTEtIsSameAsOpBase<0, 0>>,
-    AllTypesMatch<["dest", "res"]>]>,
+    AllTypesMatch<["dest", "result"]>]>,
     Arguments<(ins AnyVectorOfNonZeroRank:$valueToStore, AnyVectorOfNonZeroRank:$dest, I64ArrayAttr:$offsets,
                I64ArrayAttr:$strides)>,
-    Results<(outs AnyVectorOfNonZeroRank:$res)> {
+    Results<(outs AnyVectorOfNonZeroRank:$result)> {
   let summary = "strided_slice operation";
   let description = [{
     Takes a k-D valueToStore vector, an n-D destination vector (n >= k), n-sized

Copy link
Contributor

@krzysz00 krzysz00 left a comment

Choose a reason for hiding this comment

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

We might want to add a [[deprecated]] getter for the old name, but otherwise, approved.

@banach-space banach-space merged commit 3fe6268 into llvm:main Jun 19, 2025
7 checks passed
@banach-space banach-space deleted the andrzej/vector/rename_res branch June 19, 2025 16:35
@joker-eph
Copy link
Collaborator

We might want to add a [[deprecated]] getter for the old name, but otherwise, approved.

We don't have any API stability, this seems totally unnecessary to me: it just adds clutter and churn upstream.

@banach-space
Copy link
Contributor Author

Thanks for the feedback! I wanted to keep things consistent with what I "advertised" in https://discourse.llvm.org/t/psa-vector-standardise-operand-naming. Also, this helps downstream transition, right?

I will be removing these before the LLVM 21 release - I am very mindful of the unnecessary clutter and have a TODO to not forget to clean-up:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants