Skip to content
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

[mlir][vector][nfc] Fix typos in "VectorEmulateNarrowType.cpp" #125415

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

banach-space
Copy link
Contributor

Updates emulatedVectorLoad that was introduced in #115922.
Specifically, ATM emulatedVectorLoad mixes "emulated type" and
"container type". This only became clear after #123526 in which the
concepts of "emulated" and "container" types were introduced.

This is an NFC change and simply updates the variable naming.

Updates `emulatedVectorLoad` that was introduced in llvm#115922.
Specifically, ATM `emulatedVectorLoad` mixes "emulated type" and
"container type". This only became clear after llvm#123526 in which the
concepts of "emulated" and "container" types were introduced.

This is an NFC change and simply updates the variable naming.
@llvmbot
Copy link
Member

llvmbot commented Feb 2, 2025

@llvm/pr-subscribers-mlir

Author: Andrzej Warzyński (banach-space)

Changes

Updates emulatedVectorLoad that was introduced in #115922.
Specifically, ATM emulatedVectorLoad mixes "emulated type" and
"container type". This only became clear after #123526 in which the
concepts of "emulated" and "container" types were introduced.

This is an NFC change and simply updates the variable naming.


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

1 Files Affected:

  • (modified) mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp (+13-11)
diff --git a/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp b/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp
index 63365cb5446124..0d310dc8be2fe9 100644
--- a/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp
+++ b/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp
@@ -278,23 +278,25 @@ static Value dynamicallyInsertSubVector(RewriterBase &rewriter, Location loc,
   return dest;
 }
 
-/// Returns the op sequence for an emulated sub-byte data type vector load.
-/// specifically, use `emulatedElemType` for loading a vector of `origElemType`.
-/// The load location is given by `base` and `linearizedIndices`, and the
-/// load size is given by `numEmulatedElementsToLoad`.
+/// Emulate a vector load for `emulatedElemTy` using `containerElemTy`
+///
+/// Specifically, use `containerElemTy` for loading a vector of
+/// `emulatedElemTy`. The load location is given by `base` and
+/// `linearizedIndices`, and the load size is given by
+/// `numEmulatedElementsToLoad`.
 static VectorValue emulatedVectorLoad(OpBuilder &rewriter, Location loc,
                                       Value base,
                                       OpFoldResult linearizedIndices,
-                                      int64_t numEmultedElementsToLoad,
-                                      Type origElemType,
-                                      Type emulatedElemType) {
-  auto scale = emulatedElemType.getIntOrFloatBitWidth() /
-               origElemType.getIntOrFloatBitWidth();
+                                      int64_t numContainerElemsToLoad,
+                                      Type emulatedElemTy,
+                                      Type containerElemTy) {
+  auto scale = containerElemTy.getIntOrFloatBitWidth() /
+               emulatedElemTy.getIntOrFloatBitWidth();
   auto newLoad = rewriter.create<vector::LoadOp>(
-      loc, VectorType::get(numEmultedElementsToLoad, emulatedElemType), base,
+      loc, VectorType::get(numContainerElemsToLoad, containerElemTy), base,
       getValueOrCreateConstantIndexOp(rewriter, loc, linearizedIndices));
   return rewriter.create<vector::BitCastOp>(
-      loc, VectorType::get(numEmultedElementsToLoad * scale, origElemType),
+      loc, VectorType::get(numContainerElemsToLoad * scale, emulatedElemTy),
       newLoad);
 }
 

@llvmbot
Copy link
Member

llvmbot commented Feb 2, 2025

@llvm/pr-subscribers-mlir-vector

Author: Andrzej Warzyński (banach-space)

Changes

Updates emulatedVectorLoad that was introduced in #115922.
Specifically, ATM emulatedVectorLoad mixes "emulated type" and
"container type". This only became clear after #123526 in which the
concepts of "emulated" and "container" types were introduced.

This is an NFC change and simply updates the variable naming.


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

1 Files Affected:

  • (modified) mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp (+13-11)
diff --git a/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp b/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp
index 63365cb5446124..0d310dc8be2fe9 100644
--- a/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp
+++ b/mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp
@@ -278,23 +278,25 @@ static Value dynamicallyInsertSubVector(RewriterBase &rewriter, Location loc,
   return dest;
 }
 
-/// Returns the op sequence for an emulated sub-byte data type vector load.
-/// specifically, use `emulatedElemType` for loading a vector of `origElemType`.
-/// The load location is given by `base` and `linearizedIndices`, and the
-/// load size is given by `numEmulatedElementsToLoad`.
+/// Emulate a vector load for `emulatedElemTy` using `containerElemTy`
+///
+/// Specifically, use `containerElemTy` for loading a vector of
+/// `emulatedElemTy`. The load location is given by `base` and
+/// `linearizedIndices`, and the load size is given by
+/// `numEmulatedElementsToLoad`.
 static VectorValue emulatedVectorLoad(OpBuilder &rewriter, Location loc,
                                       Value base,
                                       OpFoldResult linearizedIndices,
-                                      int64_t numEmultedElementsToLoad,
-                                      Type origElemType,
-                                      Type emulatedElemType) {
-  auto scale = emulatedElemType.getIntOrFloatBitWidth() /
-               origElemType.getIntOrFloatBitWidth();
+                                      int64_t numContainerElemsToLoad,
+                                      Type emulatedElemTy,
+                                      Type containerElemTy) {
+  auto scale = containerElemTy.getIntOrFloatBitWidth() /
+               emulatedElemTy.getIntOrFloatBitWidth();
   auto newLoad = rewriter.create<vector::LoadOp>(
-      loc, VectorType::get(numEmultedElementsToLoad, emulatedElemType), base,
+      loc, VectorType::get(numContainerElemsToLoad, containerElemTy), base,
       getValueOrCreateConstantIndexOp(rewriter, loc, linearizedIndices));
   return rewriter.create<vector::BitCastOp>(
-      loc, VectorType::get(numEmultedElementsToLoad * scale, origElemType),
+      loc, VectorType::get(numContainerElemsToLoad * scale, emulatedElemTy),
       newLoad);
 }
 

Copy link
Member

@lialan lialan left a comment

Choose a reason for hiding this comment

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

LGTM!

@banach-space banach-space merged commit 978310f into llvm:main Feb 3, 2025
12 checks passed
@banach-space banach-space deleted the andrzej/fix_typos_narrow_type branch February 3, 2025 10:11
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…125415)

Updates `emulatedVectorLoad` that was introduced in llvm#115922.
Specifically, ATM `emulatedVectorLoad` mixes "emulated type" and
"container type". This only became clear after llvm#123526 in which the
concepts of "emulated" and "container" types were introduced.

This is an NFC change and simply updates the variable naming.
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.

3 participants