Skip to content

[Vectorize] Update comment of getSubdividedVectorType #107632

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

Merged
merged 2 commits into from
Sep 6, 2024

Conversation

Sterling-Augustine
Copy link
Contributor

The original comment here is wrong, as demonstrated by the included test.

Update the comment to reflect what getSubdividedVectorType actually does.

@llvmbot
Copy link
Member

llvmbot commented Sep 6, 2024

@llvm/pr-subscribers-llvm-ir

Author: None (Sterling-Augustine)

Changes

The original comment here is wrong, as demonstrated by the included test.

Update the comment to reflect what getSubdividedVectorType actually does.


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

2 Files Affected:

  • (modified) llvm/include/llvm/IR/DerivedTypes.h (+3-3)
  • (modified) llvm/unittests/IR/VectorTypesTest.cpp (+4)
diff --git a/llvm/include/llvm/IR/DerivedTypes.h b/llvm/include/llvm/IR/DerivedTypes.h
index 0c8cbe1921ac9b..7bc42ca0192e74 100644
--- a/llvm/include/llvm/IR/DerivedTypes.h
+++ b/llvm/include/llvm/IR/DerivedTypes.h
@@ -492,9 +492,9 @@ class VectorType : public Type {
     return VectorType::get(EltTy, VTy->getElementCount());
   }
 
-  // This static method returns a VectorType with a smaller number of elements
-  // of a larger type than the input element type. For example, a <16 x i8>
-  // subdivided twice would return <4 x i32>
+  // This static method returns a VectorType with a larger number of elements
+  // of a smaller type than the input element type. For example, a <16 x i8>
+  // subdivided twice would return <64 x i2>
   static VectorType *getSubdividedVectorType(VectorType *VTy, int NumSubdivs) {
     for (int i = 0; i < NumSubdivs; ++i) {
       VTy = VectorType::getDoubleElementsVectorType(VTy);
diff --git a/llvm/unittests/IR/VectorTypesTest.cpp b/llvm/unittests/IR/VectorTypesTest.cpp
index c592f809f7bf3e..9749a039fd203e 100644
--- a/llvm/unittests/IR/VectorTypesTest.cpp
+++ b/llvm/unittests/IR/VectorTypesTest.cpp
@@ -125,6 +125,10 @@ TEST(VectorTypesTest, FixedLength) {
   EltCnt = V8Int64Ty->getElementCount();
   EXPECT_EQ(EltCnt.getKnownMinValue(), 8U);
   ASSERT_FALSE(EltCnt.isScalable());
+
+  auto *SubTy = VectorType::getSubdividedVectorType(V16Int8Ty, 2);
+  EXPECT_EQ(SubTy->getElementCount(), ElementCount::getFixed(64));
+  EXPECT_TRUE(SubTy->getElementType()->isIntegerTy(2));
 }
 
 TEST(VectorTypesTest, Scalable) {

// of a larger type than the input element type. For example, a <16 x i8>
// subdivided twice would return <4 x i32>
// This static method returns a VectorType with a larger number of elements
// of a smaller type than the input element type. For example, a <16 x i8>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would change the example types to something more common like <4 x i64> instead of <16 x i8> which would get subdivided to <16 x i16>.

Same for the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

@vporpo vporpo left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@Sterling-Augustine Sterling-Augustine merged commit 941841b into llvm:main Sep 6, 2024
5 of 7 checks passed
@Sterling-Augustine Sterling-Augustine deleted the fixcomment branch September 11, 2024 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants