-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[AMDGPU][TTI] Add target hook for the custom instruction uniformity #137639
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
base: main
Are you sure you want to change the base?
[AMDGPU][TTI] Add target hook for the custom instruction uniformity #137639
Conversation
@llvm/pr-subscribers-llvm-adt @llvm/pr-subscribers-backend-amdgpu Author: Pankaj Dwivedi (PankajDwivedi-25) ChangesThis patch introduces a new target hook Currently, UniformityAnalysis categorizes instructions into a fixed set of InstructionUniformity values (Default, AlwaysUniform, NeverUniform). This hook allows targets to override and implement custom uniformity-propagation rules for such cases. Full diff: https://github.com/llvm/llvm-project/pull/137639.diff 7 Files Affected:
diff --git a/llvm/include/llvm/Analysis/TargetTransformInfo.h b/llvm/include/llvm/Analysis/TargetTransformInfo.h
index 022530dc846ea..9af5006ce9c6d 100644
--- a/llvm/include/llvm/Analysis/TargetTransformInfo.h
+++ b/llvm/include/llvm/Analysis/TargetTransformInfo.h
@@ -23,6 +23,7 @@
#include "llvm/ADT/APInt.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Uniformity.h"
#include "llvm/Analysis/IVDescriptors.h"
#include "llvm/IR/FMF.h"
#include "llvm/IR/InstrTypes.h"
@@ -1916,6 +1917,13 @@ class TargetTransformInfo {
const Function &F,
SmallVectorImpl<std::pair<StringRef, int64_t>> &LB) const;
+ /// Target can implement more complex patterns for getting Uniformity of an
+ /// instruction.Currently Uniformity analysis catagorises instructions with a
+ /// fixed set of InstructionUniformity values: Default, AlwaysUniform and
+ /// NeverUniform.
+ std::optional<InstructionUniformity>
+ getInstructionUniformity(const Instruction &I) const;
+
private:
std::unique_ptr<const TargetTransformInfoImplBase> TTIImpl;
};
diff --git a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
index 990252b1e5743..5bee462575181 100644
--- a/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
+++ b/llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
@@ -1147,6 +1147,11 @@ class TargetTransformInfoImplBase {
const Function &F,
SmallVectorImpl<std::pair<StringRef, int64_t>> &LB) const {}
+ virtual std::optional<InstructionUniformity>
+ getInstructionUniformity(const Instruction &I) const {
+ return std::nullopt;
+ }
+
protected:
// Obtain the minimum required size to hold the value (without the sign)
// In case of a vector it returns the min required size for one element.
diff --git a/llvm/lib/Analysis/TargetTransformInfo.cpp b/llvm/lib/Analysis/TargetTransformInfo.cpp
index 8548afea72964..50157a7714bf7 100644
--- a/llvm/lib/Analysis/TargetTransformInfo.cpp
+++ b/llvm/lib/Analysis/TargetTransformInfo.cpp
@@ -1476,6 +1476,11 @@ void TargetTransformInfo::collectKernelLaunchBounds(
return TTIImpl->collectKernelLaunchBounds(F, LB);
}
+std::optional<InstructionUniformity>
+TargetTransformInfo::getInstructionUniformity(const Instruction &I) const {
+ return TTIImpl->getInstructionUniformity(I);
+}
+
TargetTransformInfoImplBase::~TargetTransformInfoImplBase() = default;
TargetIRAnalysis::TargetIRAnalysis() : TTICallback(&getDefaultTTI) {}
diff --git a/llvm/lib/Analysis/UniformityAnalysis.cpp b/llvm/lib/Analysis/UniformityAnalysis.cpp
index 2101fdfacfc8f..2fc6f523139a7 100644
--- a/llvm/lib/Analysis/UniformityAnalysis.cpp
+++ b/llvm/lib/Analysis/UniformityAnalysis.cpp
@@ -35,7 +35,20 @@ template <> void llvm::GenericUniformityAnalysisImpl<SSAContext>::initialize() {
markDivergent(I);
else if (TTI->isAlwaysUniform(&I))
addUniformOverride(I);
+ else if (auto Uniformity = TTI->getInstructionUniformity(I)) {
+ switch (*Uniformity) {
+ case InstructionUniformity::AlwaysUniform:
+ addUniformOverride(I);
+ break;
+ case InstructionUniformity::NeverUniform:
+ markDivergent(I);
+ break;
+ case InstructionUniformity::Default:
+ break;
+ }
+ }
}
+
for (auto &Arg : F.args()) {
if (TTI->isSourceOfDivergence(&Arg)) {
markDivergent(&Arg);
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
index 204d3df546bbf..5c59847dfeb62 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.cpp
@@ -1422,3 +1422,24 @@ void GCNTTIImpl::collectKernelLaunchBounds(
LB.push_back({"amdgpu-waves-per-eu[0]", WavesPerEU.first});
LB.push_back({"amdgpu-waves-per-eu[1]", WavesPerEU.second});
}
+
+std::optional<InstructionUniformity>
+GCNTTIImpl::getInstructionUniformity(const Instruction &I) const {
+ if (const auto *II = dyn_cast<IntrinsicInst>(&I)) {
+ // We can define the custom rules for the intrinsics uniformity, depending
+ // on argument.
+ switch (II->getIntrinsicID()) {
+ case Intrinsic::amdgcn_permlane64:
+ // If either operand is uniform, the result is uniform.
+ for (unsigned Arg_i = 0, NumArg = II->arg_size(); Arg_i < NumArg;
+ Arg_i++) {
+ if (!isSourceOfDivergence(II->getArgOperand(Arg_i)))
+ return InstructionUniformity::AlwaysUniform;
+ }
+ return InstructionUniformity::Default;
+ default:
+ break;
+ }
+ }
+ return std::nullopt;
+}
\ No newline at end of file
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h
index f6f7bd4bfcf5b..bea0b024d745b 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPUTargetTransformInfo.h
@@ -290,6 +290,8 @@ class GCNTTIImpl final : public BasicTTIImplBase<GCNTTIImpl> {
void collectKernelLaunchBounds(
const Function &F,
SmallVectorImpl<std::pair<StringRef, int64_t>> &LB) const override;
+ std::optional<InstructionUniformity>
+ getInstructionUniformity(const Instruction &I) const override;
};
} // end namespace llvm
diff --git a/llvm/test/Analysis/UniformityAnalysis/AMDGPU/uniform_intrinsic.ll b/llvm/test/Analysis/UniformityAnalysis/AMDGPU/uniform_intrinsic.ll
new file mode 100644
index 0000000000000..4bb89516b2e81
--- /dev/null
+++ b/llvm/test/Analysis/UniformityAnalysis/AMDGPU/uniform_intrinsic.ll
@@ -0,0 +1,25 @@
+; RUN: opt -mtriple amdgcn-unknown-amdhsa -passes='print<uniformity>' -disable-output %s 2>&1 | FileCheck %s
+
+; CHECK: ALL VALUES UNIFORM
+define amdgpu_kernel void @permlane64_constant(ptr addrspace(1) %out) {
+ %v = call i32 @llvm.amdgcn.permlane64(i32 7)
+ store i32 %v, ptr addrspace(1) %out
+ ret void
+}
+
+; CHECK: ALL VALUES UNIFORM
+define amdgpu_kernel void @permlane64_uniform(ptr addrspace(1) %out, i32 %src) {
+ %v = call i32 @llvm.amdgcn.permlane64(i32 %src)
+ store i32 %v, ptr addrspace(1) %out
+ ret void
+}
+
+; CHECK: DIVERGENT: %tid = call i32 @llvm.amdgcn.workitem.id.x()
+; CHECK: DIVERGENT: %v = call i32 @llvm.amdgcn.permlane64.i32(i32 %tid)
+define amdgpu_kernel void @permlane64_nonuniform(i32 addrspace(1)* %out) {
+ %tid = call i32 @llvm.amdgcn.workitem.id.x()
+ %v = call i32 @llvm.amdgcn.permlane64(i32 %tid)
+ %out_ptr = getelementptr i32, i32 addrspace(1)* %out, i32 %tid
+ store i32 %v, i32 addrspace(1)* %out_ptr
+ ret void
+}
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what this new hook gives that the existing ones do not handle already
I think this is supposed to address #131779 (but in its current form it does not). |
Right, this is initial patch. Right now it doesn't do anything more than already there. I was looking for two things here.
|
8cca1b2
to
e4cc773
Compare
I can see the target intrinsic of this hook is currently classified as For example, I have removed one such intrinsic from the list in the .td, and now the hook can classify it as uniform if one or more of its operands are uniform. |
return InstructionUniformity::Default; | ||
|
||
if (CalledFunc->getIntrinsicID() == Intrinsic::amdgcn_permlane16) { | ||
// Check if any operand is uniform. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this code does not seem to check any operand. It only checks the new InstructionUniformity
.
if (!Uniformity || *Uniformity == InstructionUniformity::Default) | ||
markDivergent(*UserInstr); // fallback: conservative | ||
else if (*Uniformity == InstructionUniformity::Uniform) | ||
addUniformOverride(*UserInstr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No one checked if the relevant operand is uniform. How did we conclude that the result is uniform?
llvm/include/llvm/ADT/Uniformity.h
Outdated
NeverUniform | ||
NeverUniform, | ||
|
||
/// The result value is uniform because one or more of its operand are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be saying: "The result is uniform if any operand is uniform". That is unlikely to be true for all intrinsics. We need statements of the form "The result is uniform if operand N is uniform".
e4cc773
to
860b485
Compare
The way you have implemented this, For example, llvm.amdgcn.permlane16 has extra I really think we need a more flexible target hook that can implement any arbitrary logic for the uniformity of the result of an operation, based on the uniformity of each of its operands. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we already have special context dependent uniformity with the existing API? e.g. identifying math based on the work item ID
/// \brief keep track of special target intrinsics that can be proven uniform. | ||
void addSpecialUniformIntrinsic(const InstructionT &Instr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not clear to me what 'special' means. Is there a better name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term special
means here that intrinsic can be uniform
though not all of its operands are uniform.
Right, I am also not convinced of this name. will rename it later based on refined functionality.
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5 | ||
; RUN: opt -mtriple amdgcn-- -passes='print<uniformity>' -disable-output %s 2>&1 | FileCheck %s | ||
|
||
; CHECK: ALL VALUES UNIFORM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These aren't always always uniform, so need test coverage for all the conditions that would make this divergent
if (SpecialUniformIntrinsics.count(UserInstr) && | ||
isAnyOperandUniform(*UserInstr)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is too specific of a "specially uniform" check. I'd expect this operand validation to be an operation dependent property
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think as @jayfoad suggested. Probably I should let this be decided by the target. The only thing is that it would be costlier, and the decision of uniformity will be taken outside of UA, which I am not sure is acceptable?
template <> | ||
bool llvm::GenericUniformityAnalysisImpl<SSAContext>::isDivergentUse( | ||
const Use &U) const { | ||
const auto *V = U.get(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no auto
I don't get this. you mean pattern optimizations based on work item ID? I don't see any such API existing at the IR level. |
The only way I see to achieve this is by passing the Uniformity of the operands to Targets and letting them define the custom rule. My concern is that it would be a bit expensive, also the decision of uniformity will be taken outside of UA. Let me know if you guys have any thoughts on how best this can be implemented? |
Yes we have some special known-uniform cases in GCNTTIImpl::isAlwaysUniform, but that code does not have access to the uniformity of the operands, so it cannot implement rules like "result is uniform if any operand is uniform". |
Exactly, you would need some kind of target hook where the uniformity of the operands is passed in, or where the target hook can call back into UA to query the uniformity of operands.
I don't see why it would be too expensive. If it is then we should just give up on trying to implement this feature. |
We believe it will be expensive because there's a virtual function call happening every time the UA (re)visits an intrinsic call. The approach we are exploring now is more static ... the target hook can return an enum encoding the "uniformity policy" of the intrinsic, which will be cached. Then the UA can interpret the policy every time it visits an intrinsic. That way, all the queries and decisions taken for uniformity stay within the UA implementation, and the virtual function is called only once when initializing the UA. |
Well, OK, but I think it needs to be at least flexible enough to handle cases like ""result is uniform if either of the first two operands are uniform", for intrinsics like llvm.amdgcn.permlane16. |
See: #131779
This patch introduces a new target hook
isSpecialUniformIntrinsic()
inTargetTransformInfo
, enabling targets to describe more complex relationships between operand uniformity and instruction uniformity.Currently, all the users of the divergent values are marked as divergent conservatively; instead target can override the hook
isSpecialUniformIntrinsic()
to capture the operand-dependent uniformity.