[AMDGPU] Move kernarg preload logic to separate pass #130434
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-globalisel @llvm/pr-subscribers-backend-amdgpu Author: Austin Kerbow (kerbowa) ChangesMoves kernarg preload logic to its own module pass. Cloned function Patch is 74.77 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/130434.diff 19 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPU.h b/llvm/lib/Target/AMDGPU/AMDGPU.h
index 57297288eecb4..4c26f148310e2 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPU.h
+++ b/llvm/lib/Target/AMDGPU/AMDGPU.h
@@ -65,6 +65,7 @@ ModulePass *createAMDGPULowerBufferFatPointersPass();
FunctionPass *createSIModeRegisterPass();
FunctionPass *createGCNPreRAOptimizationsLegacyPass();
FunctionPass *createAMDGPUPreloadKernArgPrologLegacyPass();
+ModulePass *createAMDGPUPreloadKernelArgumentsLegacyPass(const TargetMachine *);
struct AMDGPUSimplifyLibCallsPass : PassInfoMixin<AMDGPUSimplifyLibCallsPass> {
AMDGPUSimplifyLibCallsPass() {}
@@ -234,6 +235,9 @@ extern char &GCNRegPressurePrinterID;
void initializeAMDGPUPreloadKernArgPrologLegacyPass(PassRegistry &);
extern char &AMDGPUPreloadKernArgPrologLegacyID;
+void initializeAMDGPUPreloadKernelArgumentsLegacyPass(PassRegistry &);
+extern char &AMDGPUPreloadKernelArgumentsLegacyID;
+
// Passes common to R600 and SI
FunctionPass *createAMDGPUPromoteAlloca();
void initializeAMDGPUPromoteAllocaPass(PassRegistry&);
@@ -345,6 +349,17 @@ class AMDGPUAttributorPass : public PassInfoMixin<AMDGPUAttributorPass> {
PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
};
+class AMDGPUPreloadKernelArgumentsPass
+ : public PassInfoMixin<AMDGPUPreloadKernelArgumentsPass> {
+ const AMDGPUTargetMachine &TM;
+
+public:
+ explicit AMDGPUPreloadKernelArgumentsPass(const AMDGPUTargetMachine &TM)
+ : TM(TM) {}
+
+ PreservedAnalyses run(Module &M, ModuleAnalysisManager &AM);
+};
+
class AMDGPUAnnotateUniformValuesPass
: public PassInfoMixin<AMDGPUAnnotateUniformValuesPass> {
public:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
index cfff66fa07f98..bbfa88a3fe872 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp
@@ -28,10 +28,6 @@ void initializeCycleInfoWrapperPassPass(PassRegistry &);
using namespace llvm;
-static cl::opt<unsigned> KernargPreloadCount(
- "amdgpu-kernarg-preload-count",
- cl::desc("How many kernel arguments to preload onto SGPRs"), cl::init(0));
-
static cl::opt<unsigned> IndirectCallSpecializationThreshold(
"amdgpu-indirect-call-specialization-threshold",
cl::desc(
@@ -1319,21 +1315,6 @@ struct AAAMDGPUNoAGPR
const char AAAMDGPUNoAGPR::ID = 0;
-static void addPreloadKernArgHint(Function &F, TargetMachine &TM) {
- const GCNSubtarget &ST = TM.getSubtarget<GCNSubtarget>(F);
- for (unsigned I = 0;
- I < F.arg_size() &&
- I < std::min(KernargPreloadCount.getValue(), ST.getMaxNumUserSGPRs());
- ++I) {
- Argument &Arg = *F.getArg(I);
- // Check for incompatible attributes.
- if (Arg.hasByRefAttr() || Arg.hasNestAttr())
- break;
-
- Arg.addAttr(Attribute::InReg);
- }
-}
-
static bool runImpl(Module &M, AnalysisGetter &AG, TargetMachine &TM,
AMDGPUAttributorOptions Options) {
SetVector<Function *> Functions;
@@ -1383,8 +1364,6 @@ static bool runImpl(Module &M, AnalysisGetter &AG, TargetMachine &TM,
if (!AMDGPU::isEntryFunctionCC(CC)) {
A.getOrCreateAAFor<AAAMDFlatWorkGroupSize>(IRPosition::function(*F));
A.getOrCreateAAFor<AAAMDWavesPerEU>(IRPosition::function(*F));
- } else if (CC == CallingConv::AMDGPU_KERNEL) {
- addPreloadKernArgHint(*F, TM);
}
for (auto &I : instructions(F)) {
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp b/llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp
index 09412d1b0f1cc..d2dd6869f1070 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULowerKernelArguments.cpp
@@ -27,231 +27,6 @@ using namespace llvm;
namespace {
-class PreloadKernelArgInfo {
-private:
- Function &F;
- const GCNSubtarget &ST;
- unsigned NumFreeUserSGPRs;
-
- enum HiddenArg : unsigned {
- HIDDEN_BLOCK_COUNT_X,
- HIDDEN_BLOCK_COUNT_Y,
- HIDDEN_BLOCK_COUNT_Z,
- HIDDEN_GROUP_SIZE_X,
- HIDDEN_GROUP_SIZE_Y,
- HIDDEN_GROUP_SIZE_Z,
- HIDDEN_REMAINDER_X,
- HIDDEN_REMAINDER_Y,
- HIDDEN_REMAINDER_Z,
- END_HIDDEN_ARGS
- };
-
- // Stores information about a specific hidden argument.
- struct HiddenArgInfo {
- // Offset in bytes from the location in the kernearg segment pointed to by
- // the implicitarg pointer.
- uint8_t Offset;
- // The size of the hidden argument in bytes.
- uint8_t Size;
- // The name of the hidden argument in the kernel signature.
- const char *Name;
- };
-
- static constexpr HiddenArgInfo HiddenArgs[END_HIDDEN_ARGS] = {
- {0, 4, "_hidden_block_count_x"}, {4, 4, "_hidden_block_count_y"},
- {8, 4, "_hidden_block_count_z"}, {12, 2, "_hidden_group_size_x"},
- {14, 2, "_hidden_group_size_y"}, {16, 2, "_hidden_group_size_z"},
- {18, 2, "_hidden_remainder_x"}, {20, 2, "_hidden_remainder_y"},
- {22, 2, "_hidden_remainder_z"}};
-
- static HiddenArg getHiddenArgFromOffset(unsigned Offset) {
- for (unsigned I = 0; I < END_HIDDEN_ARGS; ++I)
- if (HiddenArgs[I].Offset == Offset)
- return static_cast<HiddenArg>(I);
-
- return END_HIDDEN_ARGS;
- }
-
- static Type *getHiddenArgType(LLVMContext &Ctx, HiddenArg HA) {
- if (HA < END_HIDDEN_ARGS)
- return Type::getIntNTy(Ctx, HiddenArgs[HA].Size * 8);
-
- llvm_unreachable("Unexpected hidden argument.");
- }
-
- static const char *getHiddenArgName(HiddenArg HA) {
- if (HA < END_HIDDEN_ARGS) {
- return HiddenArgs[HA].Name;
- }
- llvm_unreachable("Unexpected hidden argument.");
- }
-
- // Clones the function after adding implicit arguments to the argument list
- // and returns the new updated function. Preloaded implicit arguments are
- // added up to and including the last one that will be preloaded, indicated by
- // LastPreloadIndex. Currently preloading is only performed on the totality of
- // sequential data from the kernarg segment including implicit (hidden)
- // arguments. This means that all arguments up to the last preloaded argument
- // will also be preloaded even if that data is unused.
- Function *cloneFunctionWithPreloadImplicitArgs(unsigned LastPreloadIndex) {
- FunctionType *FT = F.getFunctionType();
- LLVMContext &Ctx = F.getParent()->getContext();
- SmallVector<Type *, 16> FTypes(FT->param_begin(), FT->param_end());
- for (unsigned I = 0; I <= LastPreloadIndex; ++I)
- FTypes.push_back(getHiddenArgType(Ctx, HiddenArg(I)));
-
- FunctionType *NFT =
- FunctionType::get(FT->getReturnType(), FTypes, FT->isVarArg());
- Function *NF =
- Function::Create(NFT, F.getLinkage(), F.getAddressSpace(), F.getName());
-
- NF->copyAttributesFrom(&F);
- NF->copyMetadata(&F, 0);
- NF->setIsNewDbgInfoFormat(F.IsNewDbgInfoFormat);
-
- F.getParent()->getFunctionList().insert(F.getIterator(), NF);
- NF->takeName(&F);
- NF->splice(NF->begin(), &F);
-
- Function::arg_iterator NFArg = NF->arg_begin();
- for (Argument &Arg : F.args()) {
- Arg.replaceAllUsesWith(&*NFArg);
- NFArg->takeName(&Arg);
- ++NFArg;
- }
-
- AttrBuilder AB(Ctx);
- AB.addAttribute(Attribute::InReg);
- AB.addAttribute("amdgpu-hidden-argument");
- AttributeList AL = NF->getAttributes();
- for (unsigned I = 0; I <= LastPreloadIndex; ++I) {
- AL = AL.addParamAttributes(Ctx, NFArg->getArgNo(), AB);
- NFArg++->setName(getHiddenArgName(HiddenArg(I)));
- }
-
- NF->setAttributes(AL);
- F.replaceAllUsesWith(NF);
- F.setCallingConv(CallingConv::C);
- F.clearMetadata();
-
- return NF;
- }
-
-public:
- PreloadKernelArgInfo(Function &F, const GCNSubtarget &ST) : F(F), ST(ST) {
- setInitialFreeUserSGPRsCount();
- }
-
- // Returns the maximum number of user SGPRs that we have available to preload
- // arguments.
- void setInitialFreeUserSGPRsCount() {
- GCNUserSGPRUsageInfo UserSGPRInfo(F, ST);
- NumFreeUserSGPRs = UserSGPRInfo.getNumFreeUserSGPRs();
- }
-
- bool tryAllocPreloadSGPRs(unsigned AllocSize, uint64_t ArgOffset,
- uint64_t LastExplicitArgOffset) {
- // Check if this argument may be loaded into the same register as the
- // previous argument.
- if (ArgOffset - LastExplicitArgOffset < 4 &&
- !isAligned(Align(4), ArgOffset))
- return true;
-
- // Pad SGPRs for kernarg alignment.
- ArgOffset = alignDown(ArgOffset, 4);
- unsigned Padding = ArgOffset - LastExplicitArgOffset;
- unsigned PaddingSGPRs = alignTo(Padding, 4) / 4;
- unsigned NumPreloadSGPRs = alignTo(AllocSize, 4) / 4;
- if (NumPreloadSGPRs + PaddingSGPRs > NumFreeUserSGPRs)
- return false;
-
- NumFreeUserSGPRs -= (NumPreloadSGPRs + PaddingSGPRs);
- return true;
- }
-
- // Try to allocate SGPRs to preload implicit kernel arguments.
- void tryAllocImplicitArgPreloadSGPRs(uint64_t ImplicitArgsBaseOffset,
- uint64_t LastExplicitArgOffset,
- IRBuilder<> &Builder) {
- Function *ImplicitArgPtr = Intrinsic::getDeclarationIfExists(
- F.getParent(), Intrinsic::amdgcn_implicitarg_ptr);
- if (!ImplicitArgPtr)
- return;
-
- const DataLayout &DL = F.getParent()->getDataLayout();
- // Pair is the load and the load offset.
- SmallVector<std::pair<LoadInst *, unsigned>, 4> ImplicitArgLoads;
- for (auto *U : ImplicitArgPtr->users()) {
- Instruction *CI = dyn_cast<Instruction>(U);
- if (!CI || CI->getParent()->getParent() != &F)
- continue;
-
- for (auto *U : CI->users()) {
- int64_t Offset = 0;
- auto *Load = dyn_cast<LoadInst>(U); // Load from ImplicitArgPtr?
- if (!Load) {
- if (GetPointerBaseWithConstantOffset(U, Offset, DL) != CI)
- continue;
-
- Load = dyn_cast<LoadInst>(*U->user_begin()); // Load from GEP?
- }
-
- if (!Load || !Load->isSimple())
- continue;
-
- // FIXME: Expand to handle 64-bit implicit args and large merged loads.
- LLVMContext &Ctx = F.getParent()->getContext();
- Type *LoadTy = Load->getType();
- HiddenArg HA = getHiddenArgFromOffset(Offset);
- if (HA == END_HIDDEN_ARGS || LoadTy != getHiddenArgType(Ctx, HA))
- continue;
-
- ImplicitArgLoads.push_back(std::make_pair(Load, Offset));
- }
- }
-
- if (ImplicitArgLoads.empty())
- return;
-
- // Allocate loads in order of offset. We need to be sure that the implicit
- // argument can actually be preloaded.
- std::sort(ImplicitArgLoads.begin(), ImplicitArgLoads.end(), less_second());
-
- // If we fail to preload any implicit argument we know we don't have SGPRs
- // to preload any subsequent ones with larger offsets. Find the first
- // argument that we cannot preload.
- auto *PreloadEnd = std::find_if(
- ImplicitArgLoads.begin(), ImplicitArgLoads.end(),
- [&](const std::pair<LoadInst *, unsigned> &Load) {
- unsigned LoadSize = DL.getTypeStoreSize(Load.first->getType());
- unsigned LoadOffset = Load.second;
- if (!tryAllocPreloadSGPRs(LoadSize,
- LoadOffset + ImplicitArgsBaseOffset,
- LastExplicitArgOffset))
- return true;
-
- LastExplicitArgOffset =
- ImplicitArgsBaseOffset + LoadOffset + LoadSize;
- return false;
- });
-
- if (PreloadEnd == ImplicitArgLoads.begin())
- return;
-
- unsigned LastHiddenArgIndex = getHiddenArgFromOffset(PreloadEnd[-1].second);
- Function *NF = cloneFunctionWithPreloadImplicitArgs(LastHiddenArgIndex);
- assert(NF);
- for (const auto *I = ImplicitArgLoads.begin(); I != PreloadEnd; ++I) {
- LoadInst *LoadInst = I->first;
- unsigned LoadOffset = I->second;
- unsigned HiddenArgIndex = getHiddenArgFromOffset(LoadOffset);
- unsigned Index = NF->arg_size() - LastHiddenArgIndex + HiddenArgIndex - 1;
- Argument *Arg = NF->getArg(Index);
- LoadInst->replaceAllUsesWith(Arg);
- }
- }
-};
-
class AMDGPULowerKernelArguments : public FunctionPass {
public:
static char ID;
@@ -311,10 +86,6 @@ static bool lowerKernelArguments(Function &F, const TargetMachine &TM) {
Attribute::getWithDereferenceableBytes(Ctx, TotalKernArgSize));
uint64_t ExplicitArgOffset = 0;
- // Preloaded kernel arguments must be sequential.
- bool InPreloadSequence = true;
- PreloadKernelArgInfo PreloadInfo(F, ST);
-
for (Argument &Arg : F.args()) {
const bool IsByRef = Arg.hasByRefAttr();
Type *ArgTy = IsByRef ? Arg.getParamByRefType() : Arg.getType();
@@ -325,25 +96,10 @@ static bool lowerKernelArguments(Function &F, const TargetMachine &TM) {
uint64_t AllocSize = DL.getTypeAllocSize(ArgTy);
uint64_t EltOffset = alignTo(ExplicitArgOffset, ABITypeAlign) + BaseOffset;
- uint64_t LastExplicitArgOffset = ExplicitArgOffset;
ExplicitArgOffset = alignTo(ExplicitArgOffset, ABITypeAlign) + AllocSize;
- // Guard against the situation where hidden arguments have already been
- // lowered and added to the kernel function signiture, i.e. in a situation
- // where this pass has run twice.
- if (Arg.hasAttribute("amdgpu-hidden-argument"))
- break;
-
- // Try to preload this argument into user SGPRs.
- if (Arg.hasInRegAttr() && InPreloadSequence && ST.hasKernargPreload() &&
- !Arg.getType()->isAggregateType())
- if (PreloadInfo.tryAllocPreloadSGPRs(AllocSize, EltOffset,
- LastExplicitArgOffset))
- continue;
-
- InPreloadSequence = false;
-
- if (Arg.use_empty())
+ // Skip inreg arguments which should be preloaded.
+ if (Arg.use_empty() || Arg.hasInRegAttr())
continue;
// If this is byval, the loads are already explicit in the function. We just
@@ -483,14 +239,6 @@ static bool lowerKernelArguments(Function &F, const TargetMachine &TM) {
KernArgSegment->addRetAttr(
Attribute::getWithAlignment(Ctx, std::max(KernArgBaseAlign, MaxAlign)));
- if (InPreloadSequence) {
- uint64_t ImplicitArgsBaseOffset =
- alignTo(ExplicitArgOffset, ST.getAlignmentForImplicitArgPtr()) +
- BaseOffset;
- PreloadInfo.tryAllocImplicitArgPreloadSGPRs(ImplicitArgsBaseOffset,
- ExplicitArgOffset, Builder);
- }
-
return true;
}
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def b/llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def
index 1050855176c04..ca9a4877907c8 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPassRegistry.def
@@ -29,6 +29,7 @@ MODULE_PASS("amdgpu-printf-runtime-binding", AMDGPUPrintfRuntimeBindingPass())
MODULE_PASS("amdgpu-remove-incompatible-functions", AMDGPURemoveIncompatibleFunctionsPass(*this))
MODULE_PASS("amdgpu-sw-lower-lds", AMDGPUSwLowerLDSPass(*this))
MODULE_PASS("amdgpu-unify-metadata", AMDGPUUnifyMetadataPass())
+MODULE_PASS("amdgpu-preload-kernel-arguments", AMDGPUPreloadKernelArgumentsPass(*this))
#undef MODULE_PASS
#ifndef MODULE_PASS_WITH_PARAMS
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUPreloadKernelArguments.cpp b/llvm/lib/Target/AMDGPU/AMDGPUPreloadKernelArguments.cpp
new file mode 100644
index 0000000000000..2b2f649836ebe
--- /dev/null
+++ b/llvm/lib/Target/AMDGPU/AMDGPUPreloadKernelArguments.cpp
@@ -0,0 +1,358 @@
+//===- AMDGPUPreloadKernelArguments.cpp - Preload Kernel Arguments --------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+/// \file This pass preloads kernel arguments into user_data SGPRs before kernel
+/// execution begins. The number of registers available for preloading depends
+/// on the number of free user SGPRs, up to the hardware's maximum limit.
+/// Implicit arguments enabled in the kernel descriptor are allocated first,
+/// followed by SGPRs used for preloaded kernel arguments. (Reference:
+/// https://llvm.org/docs/AMDGPUUsage.html#initial-kernel-execution-state)
+/// Additionally, hidden kernel arguments may be preloaded, in which case they
+/// are appended to the kernel signature after explicit arguments. Preloaded
+/// arguments will be marked with `inreg`.
+//
+//===----------------------------------------------------------------------===//
+
+#include "AMDGPU.h"
+#include "AMDGPUTargetMachine.h"
+#include "llvm/Analysis/ValueTracking.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/IntrinsicsAMDGPU.h"
+#include "llvm/IR/Module.h"
+#include "llvm/IR/PassManager.h"
+#include "llvm/IR/Verifier.h"
+#include "llvm/Pass.h"
+
+#define DEBUG_TYPE "amdgpu-preload-kernel-arguments"
+
+using namespace llvm;
+
+static cl::opt<unsigned> KernargPreloadCount(
+ "amdgpu-kernarg-preload-count",
+ cl::desc("How many kernel arguments to preload onto SGPRs"), cl::init(0));
+
+namespace {
+
+class AMDGPUPreloadKernelArgumentsLegacy : public ModulePass {
+ const AMDGPUTargetMachine *TM;
+
+public:
+ static char ID;
+ explicit AMDGPUPreloadKernelArgumentsLegacy(
+ const AMDGPUTargetMachine *TM = nullptr);
+
+ StringRef getPassName() const override {
+ return "AMDGPU Preload Kernel Arguments";
+ }
+
+ bool runOnModule(Module &M) override;
+};
+
+class PreloadKernelArgInfo {
+private:
+ Function &F;
+ const GCNSubtarget &ST;
+ unsigned NumFreeUserSGPRs;
+
+ enum HiddenArg : unsigned {
+ HIDDEN_BLOCK_COUNT_X,
+ HIDDEN_BLOCK_COUNT_Y,
+ HIDDEN_BLOCK_COUNT_Z,
+ HIDDEN_GROUP_SIZE_X,
+ HIDDEN_GROUP_SIZE_Y,
+ HIDDEN_GROUP_SIZE_Z,
+ HIDDEN_REMAINDER_X,
+ HIDDEN_REMAINDER_Y,
+ HIDDEN_REMAINDER_Z,
+ END_HIDDEN_ARGS
+ };
+
+ // Stores information about a specific hidden argument.
+ struct HiddenArgInfo {
+ // Offset in bytes from the location in the kernearg segment pointed to by
+ // the implicitarg pointer.
+ uint8_t Offset;
+ // The size of the hidden argument in bytes.
+ uint8_t Size;
+ // The name of the hidden argument in the kernel signature.
+ const char *Name;
+ };
+
+ static constexpr HiddenArgInfo HiddenArgs[END_HIDDEN_ARGS] = {
+ {0, 4, "_hidden_block_count_x"}, {4, 4, "_hidden_block_count_y"},
+ {8, 4, "_hidden_block_count_z"}, {12, 2, "_hidden_group_size_x"},
+ {14, 2, "_hidden_group_size_y"}, {16, 2, "_hidden_group_size_z"},
+ {18, 2, "_hidden_remainder_x"}, {20, 2, "_hidden_remainder_y"},
+ {22, 2, "_hidden_remainder_z"}};
+
+ static HiddenArg getHiddenArgFromOffset(unsigned Offset) {
+ for (unsigned I = 0; I < END_HIDDEN_ARGS; ++I)
+ if (HiddenArgs[I].Offset == Offset)
+ return static_cast<HiddenArg>(I);
+
+ return END_HIDDEN_ARGS;
+ }
+
+ static Type *getHiddenArgType(LLVMContext &Ctx, HiddenArg HA) {
+ if (HA < END_HIDDEN_ARGS)
+ return Type::getIntNTy(Ctx, HiddenArgs[HA].Size * 8);
+
+ llvm_unreachable("Unexpected hidden argument.");
+ }
+
+ static const char *getHiddenArgName(HiddenArg HA) {
+ if (HA < END_HIDDEN_ARGS) {
+ return HiddenArgs[HA].Name;
+ }
+ llvm_unreachable("Unexpected hidden argument.");
+ }
+
+ // Clones the function after adding implicit arguments to the argument list
+ // and returns the new updated function. Preloaded implicit arguments are
+ // added up to and including the last one that will be preloaded, indicated by
+ // LastPreloadIndex. Currently preloading is only performed on the totality of
+ // sequential data from the kernarg segment including implicit (hidden)
+ // arguments. This means that all arguments up to the last preloaded argument
+ // will also be preloaded even if that data is unused.
+ Function *cloneFunctionWithPreloadImplicitArgs(unsigned LastPreloadIndex) {
+ FunctionType *FT = F.getFunctionType();
+ LLVMContext &Ctx = F.getParent()->getContext();
+ SmallVector<Type *, 16> FTypes(FT->param_begin(), FT->param_end());
+ for (unsigned I = 0; I <= LastPreloadIndex; ++I)
+ FTypes.push_back(getHiddenArgType(Ctx, HiddenArg(I)));
+
+ FunctionType *NFT =
+ FunctionType::get(FT->getReturnType(), FTypes, FT->isVarArg());
+ Function *NF =
+ Functio...
[truncated]
|
shiltian
reviewed
Mar 9, 2025
shiltian
reviewed
Mar 9, 2025
kerbowa
force-pushed
the
users/kerbowa/preload-kernarg-pass
branch
from
805e24c to
a23a8e4
Compare
|
If anyone has opinions about the best approach for this please let me know because we need to do something. There are three PRs up that are achieving the same end, but I don't want to just rebase/update all three. At different points I've seen interest in each approach based on discussion in the PRs. |
|
I think this is the best way to handle it. Moving it to the attributor isn't acceptable to me, at least based on what we have right now. I'd need to clearly see what benefits we get from running it iteratively, and how it would actually work if we were to make it part of the attributor. |
kerbowa
force-pushed
the
users/kerbowa/preload-kernarg-pass
branch
from
a23a8e4 to
2e0ce8a
Compare
cdevadas
reviewed
May 5, 2025
cdevadas
approved these changes
May 8, 2025
Comment on lines
110
to
112
| if (HA < END_HIDDEN_ARGS) { | ||
| return HiddenArgs[HA].Name; | ||
| } |
There was a problem hiding this comment.
Suggested change
| if (HA < END_HIDDEN_ARGS) { | |
| return HiddenArgs[HA].Name; | |
| } | |
| if (HA < END_HIDDEN_ARGS) | |
| return HiddenArgs[HA].Name; |
| @@ -1317,6 +1318,10 @@ void AMDGPUPassConfig::addIRPasses() { | |||
| } | |||
|
|
|||
| void AMDGPUPassConfig::addCodeGenPrepare() { | |||
| if (TM->getTargetTriple().getArch() == Triple::amdgcn && | |||
There was a problem hiding this comment.
I don't know why we need the TargetTriple check here. If the execution reaches here, the compilation is already for amdgcn triple and not for r600. Anyways, it is unrelated to this PR.
There was a problem hiding this comment.
Realized that it is the common base class for both amdgcn and r600.
There was a problem hiding this comment.
We have isAMDGCN() so better to use that.
|
See if @shiltian has any concerns before merging it. |
arsenm
approved these changes
May 8, 2025
Comment on lines
+36
to
+38
| static cl::opt<unsigned> KernargPreloadCount( | ||
| "amdgpu-kernarg-preload-count", | ||
| cl::desc("How many kernel arguments to preload onto SGPRs"), cl::init(0)); |
There was a problem hiding this comment.
Can make this a pass parameter in a follow up
| namespace { | ||
|
|
||
| class AMDGPUPreloadKernelArgumentsLegacy : public ModulePass { | ||
| const AMDGPUTargetMachine *TM; |
There was a problem hiding this comment.
Suggested change
| const AMDGPUTargetMachine *TM; | |
| const GCNTargetMachine *TM; |
| public: | ||
| static char ID; | ||
| explicit AMDGPUPreloadKernelArgumentsLegacy( | ||
| const AMDGPUTargetMachine *TM = nullptr); |
There was a problem hiding this comment.
Suggested change
| const AMDGPUTargetMachine *TM = nullptr); | |
| const GCNTargetMachine *TM = nullptr); |
Comment on lines
+1173
to
+1174
| ; GCN-O3-NEXT: AMDGPU Preload Kernel Arguments | ||
| ; GCN-O3-NEXT: FunctionPass Manager |
There was a problem hiding this comment.
Can this be moved up with the other module passes, or is there some benefit to running the other function passes first
krzysz00
reviewed
May 8, 2025
| @@ -523,6 +525,8 @@ | |||
| ; GCN-O1-OPTS-NEXT: Scalarize Masked Memory Intrinsics | |||
| ; GCN-O1-OPTS-NEXT: Expand reduction intrinsics | |||
| ; GCN-O1-OPTS-NEXT: Early CSE | |||
| ; GCN-O1-OPTS-NEXT: AMDGPU Preload Kernel Arguments | |||
There was a problem hiding this comment.
Quick side question: does this pass need to run before the main CodeGenPrepare?
If not ... I have a plan to move AMDGPULowerBufferFatPointers after LoadStoreVectorizer, and, given the desire for buffer pointer alignment reform, might want to move kernel argument lowering after that so we can see function arguments as themselves.
Not exactly relevant or blocking, just thought I'd check on this
There was a problem hiding this comment.
I'd expect this to benefit from maximally canonical IR and run a bit earlier
There was a problem hiding this comment.
The pass mostly wants to run before function attributes are finalized that may influence the number of available user SGPRs. It also needs to be before LowerKernelArguments. Beyond that there may be some small benefit to lowering the hidden arguments earlier, but only if it doesn't run into the first issue.
Moves kernarg preload logic to its own module pass. Cloned function declarations are removed when preloading hidden arguments. The inreg attribute is now added in this pass instead of AMDGPUAttributor. The rest of the logic is copied from AMDGPULowerKernelArguments which now only check whether an arguments is marked inreg to avoid replacing direct uses of preloaded arguments. This change requires test updates to remove inreg from lit tests with kernels that don't actually want preloading.
kerbowa
force-pushed
the
users/kerbowa/preload-kernarg-pass
branch
from
e1e85fa to
bc78db2
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/8942 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/202/builds/1187 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/3528 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Moves kernarg preload logic to its own module pass. Cloned function
declarations are removed when preloading hidden arguments. The inreg
attribute is now added in this pass instead of AMDGPUAttributor. The
rest of the logic is copied from AMDGPULowerKernelArguments which now
only check whether an arguments is marked inreg to avoid replacing
direct uses of preloaded arguments. This change requires test updates to
remove inreg from lit tests with kernels that don't actually want
preloading.