[AMDGPU] Handle memcpy()-like ops in LowerBufferFatPointers #126621
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
krzysz00
requested review from
jayfoad,
arsenm,
MaheshRavishankar and
shiltian
|
@llvm/pr-subscribers-backend-amdgpu Author: Krzysztof Drewniak (krzysz00) ChangesSince LowerBufferFatPointers runs before codegenprepare, which normally handles unsupported memcpy()s,, and since you can't have a Additionally, though they're unlikely to be used, this commit adds support for memset(). This commit doesn't implement writing direct-to-LDS loads as the intrinsics, but leaves the option in the future. [AMDGPU][LowerBufferFatPointers] Support memcpy(), memset() Patch is 318.81 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/126621.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp b/llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp
index ccb874e6a934e70..e4120ee79bd4b8c 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPULowerBufferFatPointers.cpp
@@ -45,16 +45,16 @@
//
// This pass proceeds in three main phases:
//
-// ## Rewriting loads and stores of p7
+// ## Rewriting loads and stores of p7 and memcpy()-like handling
//
// The first phase is to rewrite away all loads and stors of `ptr addrspace(7)`,
// including aggregates containing such pointers, to ones that use `i160`. This
-// is handled by `StoreFatPtrsAsIntsVisitor` , which visits loads, stores, and
-// allocas and, if the loaded or stored type contains `ptr addrspace(7)`,
-// rewrites that type to one where the p7s are replaced by i160s, copying other
-// parts of aggregates as needed. In the case of a store, each pointer is
-// `ptrtoint`d to i160 before storing, and load integers are `inttoptr`d back.
-// This same transformation is applied to vectors of pointers.
+// is handled by `StoreFatPtrsAsIntsAndExpandMemcpyVisitor` , which visits
+// loads, stores, and allocas and, if the loaded or stored type contains `ptr
+// addrspace(7)`, rewrites that type to one where the p7s are replaced by i160s,
+// copying other parts of aggregates as needed. In the case of a store, each
+// pointer is `ptrtoint`d to i160 before storing, and load integers are
+// `inttoptr`d back. This same transformation is applied to vectors of pointers.
//
// Such a transformation allows the later phases of the pass to not need
// to handle buffer fat pointers moving to and from memory, where we load
@@ -66,6 +66,10 @@
// Atomics operations on `ptr addrspace(7)` values are not suppported, as the
// hardware does not include a 160-bit atomic.
//
+// In order to save on O(N) work and to ensure that the contents type
+// legalizer correctly splits up wide loads, also unconditionally lower
+// memcpy-like intrinsics into loops here.
+//
// ## Buffer contents type legalization
//
// The underlying buffer intrinsics only support types up to 128 bits long,
@@ -231,20 +235,24 @@
#include "llvm/IR/InstIterator.h"
#include "llvm/IR/InstVisitor.h"
#include "llvm/IR/Instructions.h"
+#include "llvm/IR/IntrinsicInst.h"
#include "llvm/IR/Intrinsics.h"
#include "llvm/IR/IntrinsicsAMDGPU.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/Operator.h"
#include "llvm/IR/PatternMatch.h"
#include "llvm/IR/ReplaceConstant.h"
+#include "llvm/IR/ValueHandle.h"
#include "llvm/InitializePasses.h"
#include "llvm/Pass.h"
+#include "llvm/Support/AMDGPUAddrSpace.h"
#include "llvm/Support/Alignment.h"
#include "llvm/Support/AtomicOrdering.h"
#include "llvm/Support/Debug.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Transforms/Utils/Cloning.h"
#include "llvm/Transforms/Utils/Local.h"
+#include "llvm/Transforms/Utils/LowerMemIntrinsics.h"
#include "llvm/Transforms/Utils/ValueMapper.h"
#define DEBUG_TYPE "amdgpu-lower-buffer-fat-pointers"
@@ -431,14 +439,16 @@ namespace {
/// marshalling costs when reading or storing these values, but since placing
/// such pointers into memory is an uncommon operation at best, we feel that
/// this cost is acceptable for better performance in the common case.
-class StoreFatPtrsAsIntsVisitor
- : public InstVisitor<StoreFatPtrsAsIntsVisitor, bool> {
+class StoreFatPtrsAsIntsAndExpandMemcpyVisitor
+ : public InstVisitor<StoreFatPtrsAsIntsAndExpandMemcpyVisitor, bool> {
BufferFatPtrToIntTypeMap *TypeMap;
ValueToValueMapTy ConvertedForStore;
IRBuilder<> IRB;
+ const TargetMachine *TM;
+
// Convert all the buffer fat pointers within the input value to inttegers
// so that it can be stored in memory.
Value *fatPtrsToInts(Value *V, Type *From, Type *To, const Twine &Name);
@@ -448,8 +458,10 @@ class StoreFatPtrsAsIntsVisitor
Value *intsToFatPtrs(Value *V, Type *From, Type *To, const Twine &Name);
public:
- StoreFatPtrsAsIntsVisitor(BufferFatPtrToIntTypeMap *TypeMap, LLVMContext &Ctx)
- : TypeMap(TypeMap), IRB(Ctx) {}
+ StoreFatPtrsAsIntsAndExpandMemcpyVisitor(BufferFatPtrToIntTypeMap *TypeMap,
+ LLVMContext &Ctx,
+ const TargetMachine *TM)
+ : TypeMap(TypeMap), IRB(Ctx), TM(TM) {}
bool processFunction(Function &F);
bool visitInstruction(Instruction &I) { return false; }
@@ -457,11 +469,16 @@ class StoreFatPtrsAsIntsVisitor
bool visitLoadInst(LoadInst &LI);
bool visitStoreInst(StoreInst &SI);
bool visitGetElementPtrInst(GetElementPtrInst &I);
+
+ bool visitMemCpyInst(MemCpyInst &MCI);
+ bool visitMemMoveInst(MemMoveInst &MMI);
+ bool visitMemSetInst(MemSetInst &MSI);
+ bool visitMemSetPatternInst(MemSetPatternInst &MSPI);
};
} // namespace
-Value *StoreFatPtrsAsIntsVisitor::fatPtrsToInts(Value *V, Type *From, Type *To,
- const Twine &Name) {
+Value *StoreFatPtrsAsIntsAndExpandMemcpyVisitor::fatPtrsToInts(
+ Value *V, Type *From, Type *To, const Twine &Name) {
if (From == To)
return V;
ValueToValueMapTy::iterator Find = ConvertedForStore.find(V);
@@ -498,8 +515,8 @@ Value *StoreFatPtrsAsIntsVisitor::fatPtrsToInts(Value *V, Type *From, Type *To,
return Ret;
}
-Value *StoreFatPtrsAsIntsVisitor::intsToFatPtrs(Value *V, Type *From, Type *To,
- const Twine &Name) {
+Value *StoreFatPtrsAsIntsAndExpandMemcpyVisitor::intsToFatPtrs(
+ Value *V, Type *From, Type *To, const Twine &Name) {
if (From == To)
return V;
if (isBufferFatPtrOrVector(To)) {
@@ -531,18 +548,25 @@ Value *StoreFatPtrsAsIntsVisitor::intsToFatPtrs(Value *V, Type *From, Type *To,
return Ret;
}
-bool StoreFatPtrsAsIntsVisitor::processFunction(Function &F) {
+bool StoreFatPtrsAsIntsAndExpandMemcpyVisitor::processFunction(Function &F) {
bool Changed = false;
- // The visitors will mutate GEPs and allocas, but will push loads and stores
- // to the worklist to avoid invalidation.
+ // Process memcpy-like instructions after the main iteration because they can
+ // invalidate iterators.
+ SmallVector<WeakTrackingVH> CanBecomeLoops;
for (Instruction &I : make_early_inc_range(instructions(F))) {
- Changed |= visit(I);
+ if (isa<MemTransferInst, MemSetInst, MemSetPatternInst>(I))
+ CanBecomeLoops.push_back(&I);
+ else
+ Changed |= visit(I);
+ }
+ for (WeakTrackingVH VH : make_early_inc_range(CanBecomeLoops)) {
+ Changed |= visit(cast<Instruction>(VH));
}
ConvertedForStore.clear();
return Changed;
}
-bool StoreFatPtrsAsIntsVisitor::visitAllocaInst(AllocaInst &I) {
+bool StoreFatPtrsAsIntsAndExpandMemcpyVisitor::visitAllocaInst(AllocaInst &I) {
Type *Ty = I.getAllocatedType();
Type *NewTy = TypeMap->remapType(Ty);
if (Ty == NewTy)
@@ -551,7 +575,8 @@ bool StoreFatPtrsAsIntsVisitor::visitAllocaInst(AllocaInst &I) {
return true;
}
-bool StoreFatPtrsAsIntsVisitor::visitGetElementPtrInst(GetElementPtrInst &I) {
+bool StoreFatPtrsAsIntsAndExpandMemcpyVisitor::visitGetElementPtrInst(
+ GetElementPtrInst &I) {
Type *Ty = I.getSourceElementType();
Type *NewTy = TypeMap->remapType(Ty);
if (Ty == NewTy)
@@ -563,7 +588,7 @@ bool StoreFatPtrsAsIntsVisitor::visitGetElementPtrInst(GetElementPtrInst &I) {
return true;
}
-bool StoreFatPtrsAsIntsVisitor::visitLoadInst(LoadInst &LI) {
+bool StoreFatPtrsAsIntsAndExpandMemcpyVisitor::visitLoadInst(LoadInst &LI) {
Type *Ty = LI.getType();
Type *IntTy = TypeMap->remapType(Ty);
if (Ty == IntTy)
@@ -581,7 +606,7 @@ bool StoreFatPtrsAsIntsVisitor::visitLoadInst(LoadInst &LI) {
return true;
}
-bool StoreFatPtrsAsIntsVisitor::visitStoreInst(StoreInst &SI) {
+bool StoreFatPtrsAsIntsAndExpandMemcpyVisitor::visitStoreInst(StoreInst &SI) {
Value *V = SI.getValueOperand();
Type *Ty = V->getType();
Type *IntTy = TypeMap->remapType(Ty);
@@ -597,6 +622,47 @@ bool StoreFatPtrsAsIntsVisitor::visitStoreInst(StoreInst &SI) {
return true;
}
+bool StoreFatPtrsAsIntsAndExpandMemcpyVisitor::visitMemCpyInst(
+ MemCpyInst &MCI) {
+ // TODO: Allow memcpy.p7.p3 as a synonym for the direct-to-LDS copy, which'll
+ // need loop expansion here.
+ if (MCI.getSourceAddressSpace() != AMDGPUAS::BUFFER_FAT_POINTER &&
+ MCI.getDestAddressSpace() != AMDGPUAS::BUFFER_FAT_POINTER)
+ return false;
+ llvm::expandMemCpyAsLoop(&MCI,
+ TM->getTargetTransformInfo(*MCI.getFunction()));
+ MCI.eraseFromParent();
+ return true;
+}
+
+bool StoreFatPtrsAsIntsAndExpandMemcpyVisitor::visitMemMoveInst(
+ MemMoveInst &MMI) {
+ if (MMI.getSourceAddressSpace() != AMDGPUAS::BUFFER_FAT_POINTER &&
+ MMI.getDestAddressSpace() != AMDGPUAS::BUFFER_FAT_POINTER)
+ return false;
+ report_fatal_error(
+ "memmove() on buffer descriptors is not implemented because pointer "
+ "comparison on buffer descriptors isn't implemented\n");
+}
+
+bool StoreFatPtrsAsIntsAndExpandMemcpyVisitor::visitMemSetInst(
+ MemSetInst &MSI) {
+ if (MSI.getDestAddressSpace() != AMDGPUAS::BUFFER_FAT_POINTER)
+ return false;
+ llvm::expandMemSetAsLoop(&MSI);
+ MSI.eraseFromParent();
+ return true;
+}
+
+bool StoreFatPtrsAsIntsAndExpandMemcpyVisitor::visitMemSetPatternInst(
+ MemSetPatternInst &MSPI) {
+ if (MSPI.getDestAddressSpace() != AMDGPUAS::BUFFER_FAT_POINTER)
+ return false;
+ llvm::expandMemSetPatternAsLoop(&MSPI);
+ MSPI.eraseFromParent();
+ return true;
+}
+
namespace {
/// Convert loads/stores of types that the buffer intrinsics can't handle into
/// one ore more such loads/stores that consist of legal types.
@@ -1127,6 +1193,7 @@ bool LegalizeBufferContentTypesVisitor::visitStoreInst(StoreInst &SI) {
bool LegalizeBufferContentTypesVisitor::processFunction(Function &F) {
bool Changed = false;
+ // Note, memory transfer intrinsics won't
for (Instruction &I : make_early_inc_range(instructions(F))) {
Changed |= visit(I);
}
@@ -2072,6 +2139,12 @@ static bool isRemovablePointerIntrinsic(Intrinsic::ID IID) {
case Intrinsic::invariant_end:
case Intrinsic::launder_invariant_group:
case Intrinsic::strip_invariant_group:
+ case Intrinsic::memcpy:
+ case Intrinsic::memcpy_inline:
+ case Intrinsic::memmove:
+ case Intrinsic::memset:
+ case Intrinsic::memset_inline:
+ case Intrinsic::experimental_memset_pattern:
return true;
}
}
@@ -2322,7 +2395,8 @@ bool AMDGPULowerBufferFatPointers::run(Module &M, const TargetMachine &TM) {
/*RemoveDeadConstants=*/false, /*IncludeSelf=*/true);
}
- StoreFatPtrsAsIntsVisitor MemOpsRewrite(&IntTM, M.getContext());
+ StoreFatPtrsAsIntsAndExpandMemcpyVisitor MemOpsRewrite(&IntTM, M.getContext(),
+ &TM);
LegalizeBufferContentTypesVisitor BufferContentsTypeRewrite(DL,
M.getContext());
for (Function &F : M.functions()) {
diff --git a/llvm/test/CodeGen/AMDGPU/lower-buffer-fat-pointers-mem-transfer.ll b/llvm/test/CodeGen/AMDGPU/lower-buffer-fat-pointers-mem-transfer.ll
new file mode 100644
index 000000000000000..e6c2d1907068f52
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/lower-buffer-fat-pointers-mem-transfer.ll
@@ -0,0 +1,1730 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -mcpu=gfx900 -amdgpu-lower-buffer-fat-pointers < %s | FileCheck %s
+; RUN: opt -S -mcpu=gfx900 -passes=amdgpu-lower-buffer-fat-pointers < %s | FileCheck %s
+
+target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8"
+target triple = "amdgcn--"
+
+;; memcpy
+
+declare void @llvm.memcpy.p7.p7.i32(ptr addrspace(7), ptr addrspace(7), i32, i1)
+declare void @llvm.memcpy.p1.p7.i32(ptr addrspace(1), ptr addrspace(7), i32, i1)
+declare void @llvm.memcpy.p7.p1.i32(ptr addrspace(7), ptr addrspace(1), i32, i1)
+declare void @llvm.memcpy.p7.p7.i64(ptr addrspace(7), ptr addrspace(7), i64, i1)
+declare void @llvm.memcpy.p3.p7.i32(ptr addrspace(3), ptr addrspace(7), i32, i1)
+
+define void @memcpy_known(ptr addrspace(7) inreg %src, ptr addrspace(7) inreg %dst) {
+; CHECK-LABEL: define void @memcpy_known(
+; CHECK-SAME: { ptr addrspace(8), i32 } inreg [[SRC:%.*]], { ptr addrspace(8), i32 } inreg [[DST:%.*]]) #[[ATTR0:[0-9]+]] {
+; CHECK-NEXT: [[DST_RSRC:%.*]] = extractvalue { ptr addrspace(8), i32 } [[DST]], 0
+; CHECK-NEXT: [[DST_OFF:%.*]] = extractvalue { ptr addrspace(8), i32 } [[DST]], 1
+; CHECK-NEXT: [[SRC_RSRC:%.*]] = extractvalue { ptr addrspace(8), i32 } [[SRC]], 0
+; CHECK-NEXT: [[SRC_OFF:%.*]] = extractvalue { ptr addrspace(8), i32 } [[SRC]], 1
+; CHECK-NEXT: br label %[[LOAD_STORE_LOOP:.*]]
+; CHECK: [[LOAD_STORE_LOOP]]:
+; CHECK-NEXT: [[LOOP_INDEX:%.*]] = phi i32 [ 0, [[TMP0:%.*]] ], [ [[TMP4:%.*]], %[[LOAD_STORE_LOOP]] ]
+; CHECK-NEXT: [[TMP1:%.*]] = add i32 [[SRC_OFF]], [[LOOP_INDEX]]
+; CHECK-NEXT: [[DOTOFF_0:%.*]] = call <4 x i32> @llvm.amdgcn.raw.ptr.buffer.load.v4i32(ptr addrspace(8) align 16 [[SRC_RSRC]], i32 [[TMP1]], i32 0, i32 0)
+; CHECK-NEXT: [[DOTEXT_0:%.*]] = shufflevector <4 x i32> [[DOTOFF_0]], <4 x i32> poison, <64 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
+; CHECK-NEXT: [[DOTPARTS_0:%.*]] = shufflevector <64 x i32> poison, <64 x i32> [[DOTEXT_0]], <64 x i32> <i32 64, i32 65, i32 66, i32 67, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15, i32 16, i32 17, i32 18, i32 19, i32 20, i32 21, i32 22, i32 23, i32 24, i32 25, i32 26, i32 27, i32 28, i32 29, i32 30, i32 31, i32 32, i32 33, i32 34, i32 35, i32 36, i32 37, i32 38, i32 39, i32 40, i32 41, i32 42, i32 43, i32 44, i32 45, i32 46, i32 47, i32 48, i32 49, i32 50, i32 51, i32 52, i32 53, i32 54, i32 55, i32 56, i32 57, i32 58, i32 59, i32 60, i32 61, i32 62, i32 63>
+; CHECK-NEXT: [[DOTOFF_PTR_16:%.*]] = add nuw i32 [[TMP1]], 16
+; CHECK-NEXT: [[DOTOFF_16:%.*]] = call <4 x i32> @llvm.amdgcn.raw.ptr.buffer.load.v4i32(ptr addrspace(8) align 16 [[SRC_RSRC]], i32 [[DOTOFF_PTR_16]], i32 0, i32 0)
+; CHECK-NEXT: [[DOTEXT_4:%.*]] = shufflevector <4 x i32> [[DOTOFF_16]], <4 x i32> poison, <64 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
+; CHECK-NEXT: [[DOTPARTS_4:%.*]] = shufflevector <64 x i32> [[DOTPARTS_0]], <64 x i32> [[DOTEXT_4]], <64 x i32> <i32 0, i32 1, i32 2, i32 3, i32 64, i32 65, i32 66, i32 67, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15, i32 16, i32 17, i32 18, i32 19, i32 20, i32 21, i32 22, i32 23, i32 24, i32 25, i32 26, i32 27, i32 28, i32 29, i32 30, i32 31, i32 32, i32 33, i32 34, i32 35, i32 36, i32 37, i32 38, i32 39, i32 40, i32 41, i32 42, i32 43, i32 44, i32 45, i32 46, i32 47, i32 48, i32 49, i32 50, i32 51, i32 52, i32 53, i32 54, i32 55, i32 56, i32 57, i32 58, i32 59, i32 60, i32 61, i32 62, i32 63>
+; CHECK-NEXT: [[DOTOFF_PTR_32:%.*]] = add nuw i32 [[TMP1]], 32
+; CHECK-NEXT: [[DOTOFF_32:%.*]] = call <4 x i32> @llvm.amdgcn.raw.ptr.buffer.load.v4i32(ptr addrspace(8) align 16 [[SRC_RSRC]], i32 [[DOTOFF_PTR_32]], i32 0, i32 0)
+; CHECK-NEXT: [[DOTEXT_8:%.*]] = shufflevector <4 x i32> [[DOTOFF_32]], <4 x i32> poison, <64 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
+; CHECK-NEXT: [[DOTPARTS_8:%.*]] = shufflevector <64 x i32> [[DOTPARTS_4]], <64 x i32> [[DOTEXT_8]], <64 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 64, i32 65, i32 66, i32 67, i32 12, i32 13, i32 14, i32 15, i32 16, i32 17, i32 18, i32 19, i32 20, i32 21, i32 22, i32 23, i32 24, i32 25, i32 26, i32 27, i32 28, i32 29, i32 30, i32 31, i32 32, i32 33, i32 34, i32 35, i32 36, i32 37, i32 38, i32 39, i32 40, i32 41, i32 42, i32 43, i32 44, i32 45, i32 46, i32 47, i32 48, i32 49, i32 50, i32 51, i32 52, i32 53, i32 54, i32 55, i32 56, i32 57, i32 58, i32 59, i32 60, i32 61, i32 62, i32 63>
+; CHECK-NEXT: [[DOTOFF_PTR_48:%.*]] = add nuw i32 [[TMP1]], 48
+; CHECK-NEXT: [[DOTOFF_48:%.*]] = call <4 x i32> @llvm.amdgcn.raw.ptr.buffer.load.v4i32(ptr addrspace(8) align 16 [[SRC_RSRC]], i32 [[DOTOFF_PTR_48]], i32 0, i32 0)
+; CHECK-NEXT: [[DOTEXT_12:%.*]] = shufflevector <4 x i32> [[DOTOFF_48]], <4 x i32> poison, <64 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison>
+; CHECK-NEXT: [[DOTPARTS_12:%.*]] = shufflevector <64 x i32> [[DOTPARTS_8]], <64 x i32> [[DOTEXT_12]], <64 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, i32 10, i32 11, i32 64, i32 65, i32 66, i32 67, i32 16, i32 17, i32 18, i32 19, i32 20, i32 21, i32 22, i32 23, i32 24, i32 25, i32 26, i32 27, i32 28, i32 29, i32 30, i32 31, i32 32, i32 33, i32 34, i32 35, i32 36, i32 37, i32 38, i32 39, i32 40, i32 41, i32 42, i32 43, i32 44, i32 45, i32 46, i32 47, i32 48, i32 49, i32 50, i32 51, i32 52, i32 53, i32 54, i32 55, i32 56, i32 57, i32 58, i32 59, i32 60, i32 61, i32 62, i32 63>
+; CHECK-NEXT: [[DOTOFF_PTR_64:%.*]] = add nuw i32 [[TMP1]], 64
+; CHECK-NEXT: [[DOTOFF_64:%.*]] = call <4 x i32> @llvm.amdgcn.raw.ptr.buffer.load.v4i32(ptr addrspace(8) align 16 [[SRC_RSRC]], i32 [[DOTOFF_PTR_64]], i32 0, i32 0)
+; CHECK-NEXT: [[DOTEXT_16:%.*]] = shufflevector <4 x i32> [[DOTOFF_64]], <4 x i32> poison, <64 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 pois...
[truncated]
|
Not codegenprepare's job. Do you mean PreIselIntrinsicLowering? |
arsenm
reviewed
Feb 11, 2025
Comment on lines
+632
to
+633
| llvm::expandMemCpyAsLoop(&MCI, | ||
| TM->getTargetTransformInfo(*MCI.getFunction())); |
There was a problem hiding this comment.
I'd expect this pass to just adjust the operands of the intrinsic, and not directly expand to loop
There was a problem hiding this comment.
... there's nothing I can sensibly adjust the intrinsic to
There was a problem hiding this comment.
I mean, as mentioned in the PR, there's a special case for buffer -> LDS that's future work, but, other than that ... because ptr addrspace(8) can't carry an offset and because ptr addrspace(7) has to disappear before MIR formation (because SelectionDAG, especially because SelectionDAG doesn't have ptradd), this has to be rewritten away here
There was a problem hiding this comment.
Does the LowerBufferFatPointers pass need to run before PreISelIntrinsicLowering?
There was a problem hiding this comment.
I know it really wants to run before uniformity analysis, which I think comes before PreISel...
Plus not all the cases get expanded there |
ritter-x2a
reviewed
Feb 25, 2025
| ; CHECK-NEXT: [[TMP1:%.*]] = add i32 [[SRC_OFF]], [[LOOP_INDEX]] | ||
| ; CHECK-NEXT: [[DOTOFF_0:%.*]] = call <4 x i32> @llvm.amdgcn.raw.ptr.buffer.load.v4i32(ptr addrspace(8) align 16 [[SRC_RSRC]], i32 [[TMP1]], i32 0, i32 0) | ||
| ; CHECK-NEXT: [[DOTEXT_0:%.*]] = shufflevector <4 x i32> [[DOTOFF_0]], <4 x i32> poison, <64 x i32> <i32 0, i32 1, i32 2, i32 3, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison, i32 poison> | ||
| ; CHECK-NEXT: [[DOTPARTS_0:%.*]] = shufflevector <64 x i32> poison, <64 x i32> [[DOTEXT_0]], <64 x i32> <i32 64, i32 65, i32 66, i32 67, i32 4, i32 5, i32 6, i32 7, i32 8, i32 9, i32 10, i32 11, i32 12, i32 13, i32 14, i32 15, i32 16, i32 17, i32 18, i32 19, i32 20, i32 21, i32 22, i32 23, i32 24, i32 25, i32 26, i32 27, i32 28, i32 29, i32 30, i32 31, i32 32, i32 33, i32 34, i32 35, i32 36, i32 37, i32 38, i32 39, i32 40, i32 41, i32 42, i32 43, i32 44, i32 45, i32 46, i32 47, i32 48, i32 49, i32 50, i32 51, i32 52, i32 53, i32 54, i32 55, i32 56, i32 57, i32 58, i32 59, i32 60, i32 61, i32 62, i32 63> |
There was a problem hiding this comment.
Do these shufflevectors with the large vectors get optimized away in the backend? If not, or if they make problems in some other way, we might want to specify a different LoopLoweringType for this address space in the TTI.
There was a problem hiding this comment.
I'm guessing this struggles from the same problem as other of these late expansions, we don't get the normal middle end cleanups we would hope for
There was a problem hiding this comment.
We might want to slide an InstCombine or InstSimplify or smilar after some/all of these passes (see also the division to multiplication widget), at least on O1+ or O2+?
There was a problem hiding this comment.
Definitely don't want to run instcombine in here. Maybe we could throw one of these lowering passes in the middle end just in case. We could use the simplifying IR builder though, which would get some of the cases
There was a problem hiding this comment.
Yeah, I'll swap us over to the simplifying builder in a followup patch
There was a problem hiding this comment.
And I'll add a test that lowers some of these simpler cases down to ISA to make sure they do the right thing
There was a problem hiding this comment.
@ritter-x2a @arsenm I went and added tests for the generated assembly from memcpy(). We get the sort of load load load store store store we'd expect on gfx1100, and gfx940 still basically does the right thing bus has this weird spill that feels like a separate issue.
arsenm
approved these changes
Feb 26, 2025
Since LowerBufferFatPointers runs before codegenprepare, which
normally handles unsupported memcpy()s,, and since you can't have
a `noalias {ptr addrspace(8), i32}` becasue it crashes later passes,
manually expand memcpy()s involving buffer fat pointers to loops.
Additionally, though they're unlikely to be used, this commit adds
support for memset().
This commit doesn't implement writing direct-to-LDS loads as the
intrinsics, but leaves the option in the future.
[AMDGPU][LowerBufferFatPointers] Support memcpy(), memset()
krzysz00
force-pushed
the
memcpy-loop-p7
branch
from
be6f91a to
e4226fd
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/281 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/175/builds/13866 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/185/builds/13813 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/174/builds/13624 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/137/builds/14062 Here is the relevant piece of the build log for the reference |
kazutakahirata
added a commit
that referenced
this pull request
Feb 26, 2025
|
@krzysz00 I've revert this PR due to the multiple buildbot failures above and my local build failure. I'm happy to try your revised patch. Thanks! |
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Feb 26, 2025
…tPointers (#126621)" This reverts commit 469757e. Multiple buildbot failures have been reported: llvm/llvm-project#126621
|
@kazutakahirata Sorry about that, didn't notice the failures. Thanks for the revert! Will try to re-land soon |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/33/builds/12024 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/108/builds/9764 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/153/builds/24033 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/60/builds/20559 Here is the relevant piece of the build log for the reference |
krzysz00
added a commit
to krzysz00/llvm-project
that referenced
this pull request
Feb 27, 2025
…lvm#126621)" This reverts commit 1559a65. Fixed test (I suspect broken by unrelated change in the merge)
krzysz00
added a commit
that referenced
this pull request
Feb 27, 2025
joaosaffran
pushed a commit
to joaosaffran/llvm-project
that referenced
this pull request
Mar 3, 2025
…lvm#126621)" (llvm#129078) This reverts commit 1559a65. Fixed test (I suspect broken by unrelated change in the merge)
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.
Since LowerBufferFatPointers runs before PreISelIntrinsicLowering, which normally handles unsupported memcpy()s,, and since you can't have a
noalias {ptr addrspace(8), i32}becasue it crashes later passes, manually expand memcpy()s involving buffer fat pointers to loops.Additionally, though they're unlikely to be used, this commit adds support for memset().
This commit doesn't implement writing direct-to-LDS loads as the intrinsics, but leaves the option in the future.
[AMDGPU][LowerBufferFatPointers] Support memcpy(), memset()