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

[AMDGPU] Add a new function getIntegerVecAttribute #133271

Merged
merged 3 commits into from
Mar 27, 2025

Conversation

shiltian
Copy link
Contributor

The new function will return std::nullopt when any error occurs.

The new function will return `std::nullopt` when any error occurs.
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@shiltian shiltian requested review from jayfoad, arsenm and kosarev March 27, 2025 16:03
@llvmbot
Copy link
Member

llvmbot commented Mar 27, 2025

@llvm/pr-subscribers-backend-amdgpu

Author: Shilei Tian (shiltian)

Changes

The new function will return std::nullopt when any error occurs.


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

3 Files Affected:

  • (modified) llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp (+2-1)
  • (modified) llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp (+11-5)
  • (modified) llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h (+6-6)
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp b/llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp
index 6c8116abd8dcd..fed7a13a69bc4 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULowerKernelAttributes.cpp
@@ -107,7 +107,8 @@ static bool processUse(CallInst *CI, bool IsV5OrAbove) {
     F->getFnAttribute("uniform-work-group-size").getValueAsBool();
 
   SmallVector<unsigned> MaxNumWorkgroups =
-      AMDGPU::getIntegerVecAttribute(*F, "amdgpu-max-num-workgroups", 3);
+      AMDGPU::getIntegerVecAttribute(*F, "amdgpu-max-num-workgroups",
+                                     /*Size=*/3, /*DefaultVal=*/0);
 
   if (!HasReqdWorkGroupSize && !HasUniformWorkGroupSize &&
       none_of(MaxNumWorkgroups, [](unsigned X) { return X != 0; }))
diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
index 8a919a780bb75..c1800f50c41d9 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
@@ -1398,14 +1398,20 @@ getIntegerPairAttribute(const Function &F, StringRef Name,
 SmallVector<unsigned> getIntegerVecAttribute(const Function &F, StringRef Name,
                                              unsigned Size,
                                              unsigned DefaultVal) {
+  std::optional<SmallVector<unsigned>> R =
+      getIntegerVecAttribute(F, Name, Size);
+  return R.has_value() ? *R : SmallVector<unsigned>(Size, DefaultVal);
+}
+
+std::optional<SmallVector<unsigned>>
+getIntegerVecAttribute(const Function &F, StringRef Name, unsigned Size) {
   assert(Size > 2);
-  SmallVector<unsigned> Default(Size, DefaultVal);
 
   Attribute A = F.getFnAttribute(Name);
   if (!A.isStringAttribute())
-    return Default;
+    return std::nullopt;
 
-  SmallVector<unsigned> Vals(Size, DefaultVal);
+  SmallVector<unsigned> Vals(Size);
 
   LLVMContext &Ctx = F.getContext();
 
@@ -1417,7 +1423,7 @@ SmallVector<unsigned> getIntegerVecAttribute(const Function &F, StringRef Name,
     if (Strs.first.trim().getAsInteger(0, IntVal)) {
       Ctx.emitError("can't parse integer attribute " + Strs.first + " in " +
                     Name);
-      return Default;
+      return std::nullopt;
     }
     Vals[i] = IntVal;
     S = Strs.second;
@@ -1427,7 +1433,7 @@ SmallVector<unsigned> getIntegerVecAttribute(const Function &F, StringRef Name,
     Ctx.emitError("attribute " + Name +
                   " has incorrect number of integers; expected " +
                   llvm::utostr(Size));
-    return Default;
+    return std::nullopt;
   }
   return Vals;
 }
diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
index 184f40bccfff8..f61a99c37e669 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
@@ -958,14 +958,14 @@ getIntegerPairAttribute(const Function &F, StringRef Name,
 
 /// \returns Generate a vector of integer values requested using \p F's \p Name
 /// attribute.
-///
-/// \returns true if exactly Size (>2) number of integers are found in the
-/// attribute.
-///
-/// \returns false if any error occurs.
+/// \returns A vector of size \p Size, with all elements set to \p DefaultVal,
+/// if any error occurs. The corresponding error will also be emitted.
 SmallVector<unsigned> getIntegerVecAttribute(const Function &F, StringRef Name,
                                              unsigned Size,
-                                             unsigned DefaultVal = 0);
+                                             unsigned DefaultVal);
+/// Similar to the function above, but returns std::nullopt if any error occurs.
+std::optional<SmallVector<unsigned>>
+getIntegerVecAttribute(const Function &F, StringRef Name, unsigned Size);
 
 /// Represents the counter values to wait for in an s_waitcnt instruction.
 ///

Copy link
Collaborator

@kosarev kosarev left a comment

Choose a reason for hiding this comment

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

LGTM, if passes tests.

@shiltian shiltian force-pushed the users/shiltian/new-getIntegerPairAttribute branch from 1f6abd3 to 0a73dc5 Compare March 27, 2025 16:46
@shiltian shiltian merged commit 02b45f4 into main Mar 27, 2025
11 checks passed
@shiltian shiltian deleted the users/shiltian/new-getIntegerPairAttribute branch March 27, 2025 19:38
}

std::optional<SmallVector<unsigned>>
getIntegerVecAttribute(const Function &F, StringRef Name, unsigned Size) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Title doesn't match content, we already have getIntegerPairAttribute

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't follow. This is for getIntegerVecAttribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, the title of the PR. Yeah, it should be getIntegerVecAttribute.

@shiltian shiltian changed the title [AMDGPU] Add a new function getIntegerPairAttribute [AMDGPU] Add a new function getIntegerVecAttribute Mar 28, 2025
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.

4 participants