[SPIRV] Use OpCopyMemory for logical SPIRV memcpy

[s-perron]

This commit modifies the SPIRV instruction selector to emit OpCopyMemory
instead of OpCopyMemorySized when generating SPIRV for logical addressing.

Previously, G_MEMCPY was translated to OpCopyMemorySized, which requires an
explicit size operand. However, for logical SPIRV, the size of the pointee type
is implicitly known. This change ensures that OpCopyMemory is used, which is
more appropriate for logical SPIRV and aligns with the SPIR-V specification for
logical addressing.

[llvmbot]

@llvm/pr-subscribers-backend-spir-v

Author: Steven Perron (s-perron)

Changes

This commit modifies the SPIRV instruction selector to emit OpCopyMemory
instead of OpCopyMemorySized when generating SPIRV for logical addressing.

Previously, G_MEMCPY was translated to OpCopyMemorySized, which requires an
explicit size operand. However, for logical SPIRV, the size of the pointee type
is implicitly known. This change ensures that OpCopyMemory is used, which is
more appropriate for logical SPIRV and aligns with the SPIR-V specification for
logical addressing.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp (+94-44)
• (added) llvm/test/CodeGen/SPIRV/llvm-intrinsics/logical-memcpy.ll (+32)

diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp index d3fc08eb56cb3..89f23c25ac906 100644 --- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp @@ -149,6 +149,9 @@ class SPIRVInstructionSelector : public InstructionSelector { bool selectStackRestore(MachineInstr &I) const; bool selectMemOperation(Register ResVReg, MachineInstr &I) const; + Register getOrCreateMemSetGlobal(MachineInstr &I) const; + bool selectCopyMemory(MachineInstr &I, Register SrcReg) const; + bool selectCopyMemorySized(MachineInstr &I, Register SrcReg) const; bool selectAtomicRMW(Register ResVReg, const SPIRVType *ResType, MachineInstr &I, unsigned NewOpcode, @@ -1435,50 +1438,76 @@ bool SPIRVInstructionSelector::selectStackRestore(MachineInstr &I) const { .constrainAllUses(TII, TRI, RBI); } -bool SPIRVInstructionSelector::selectMemOperation(Register ResVReg, - MachineInstr &I) const { +Register +SPIRVInstructionSelector::getOrCreateMemSetGlobal(MachineInstr &I) const { + MachineIRBuilder MIRBuilder(I); + assert(I.getOperand(1).isReg() && I.getOperand(2).isReg()); + unsigned Val = getIConstVal(I.getOperand(1).getReg(), MRI); + unsigned Num = getIConstVal(I.getOperand(2).getReg(), MRI); + Type *ValTy = Type::getInt8Ty(I.getMF()->getFunction().getContext()); + Type *ArrTy = ArrayType::get(ValTy, Num); + SPIRVType *VarTy = GR.getOrCreateSPIRVPointerType( + ArrTy, MIRBuilder, SPIRV::StorageClass::UniformConstant); + + SPIRVType *SpvArrTy = GR.getOrCreateSPIRVType( + ArrTy, MIRBuilder, SPIRV::AccessQualifier::None, false); + Register Const = GR.getOrCreateConstIntArray(Val, Num, I, SpvArrTy, TII); + // TODO: check if we have such GV, add init, use buildGlobalVariable. + Function &CurFunction = GR.CurMF->getFunction(); + Type *LLVMArrTy = + ArrayType::get(IntegerType::get(CurFunction.getContext(), 8), Num); + // Module takes ownership of the global var. + GlobalVariable *GV = new GlobalVariable(*CurFunction.getParent(), LLVMArrTy, + true, GlobalValue::InternalLinkage, + Constant::getNullValue(LLVMArrTy)); + Register VarReg = MRI->createGenericVirtualRegister(LLT::scalar(64)); + auto MIBVar = + BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(SPIRV::OpVariable)) + .addDef(VarReg) + .addUse(GR.getSPIRVTypeID(VarTy)) + .addImm(SPIRV::StorageClass::UniformConstant) + .addUse(Const); + if (!MIBVar.constrainAllUses(TII, TRI, RBI)) + return Register(); + + GR.add(GV, MIBVar); + GR.addGlobalObject(GV, GR.CurMF, VarReg); + + buildOpDecorate(VarReg, I, TII, SPIRV::Decoration::Constant, {}); + return VarReg; +} + +bool SPIRVInstructionSelector::selectCopyMemory(MachineInstr &I, + Register SrcReg) const { MachineBasicBlock &BB = *I.getParent(); - Register SrcReg = I.getOperand(1).getReg(); - bool Result = true; - if (I.getOpcode() == TargetOpcode::G_MEMSET) { + Register DstReg = I.getOperand(0).getReg(); + SPIRVType *DstTy = GR.getSPIRVTypeForVReg(DstReg); + SPIRVType *SrcTy = GR.getSPIRVTypeForVReg(SrcReg); + if (GR.getPointeeType(DstTy) != GR.getPointeeType(SrcTy)) + report_fatal_error("OpCopyMemory requires operands to have the same type"); + uint64_t CopySize = getIConstVal(I.getOperand(2).getReg(), MRI); + SPIRVType *PointeeTy = GR.getPointeeType(DstTy); + const Type *LLVMPointeeTy = GR.getTypeForSPIRVType(PointeeTy); + if (!LLVMPointeeTy) + report_fatal_error( + "Unable to determine pointee type size for OpCopyMemory"); + const DataLayout &DL = I.getMF()->getFunction().getDataLayout(); + if (CopySize != DL.getTypeStoreSize(const_cast<Type *>(LLVMPointeeTy))) + report_fatal_error( + "OpCopyMemory requires the size to match the pointee type size"); + auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpCopyMemory)) + .addUse(DstReg) + .addUse(SrcReg); + if (I.getNumMemOperands()) { MachineIRBuilder MIRBuilder(I); - assert(I.getOperand(1).isReg() && I.getOperand(2).isReg()); - unsigned Val = getIConstVal(I.getOperand(1).getReg(), MRI); - unsigned Num = getIConstVal(I.getOperand(2).getReg(), MRI); - Type *ValTy = Type::getInt8Ty(I.getMF()->getFunction().getContext()); - Type *ArrTy = ArrayType::get(ValTy, Num); - SPIRVType *VarTy = GR.getOrCreateSPIRVPointerType( - ArrTy, MIRBuilder, SPIRV::StorageClass::UniformConstant); - - SPIRVType *SpvArrTy = GR.getOrCreateSPIRVType( - ArrTy, MIRBuilder, SPIRV::AccessQualifier::None, false); - Register Const = GR.getOrCreateConstIntArray(Val, Num, I, SpvArrTy, TII); - // TODO: check if we have such GV, add init, use buildGlobalVariable. - Function &CurFunction = GR.CurMF->getFunction(); - Type *LLVMArrTy = - ArrayType::get(IntegerType::get(CurFunction.getContext(), 8), Num); - // Module takes ownership of the global var. - GlobalVariable *GV = new GlobalVariable(*CurFunction.getParent(), LLVMArrTy, - true, GlobalValue::InternalLinkage, - Constant::getNullValue(LLVMArrTy)); - Register VarReg = MRI->createGenericVirtualRegister(LLT::scalar(64)); - auto MIBVar = - BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(SPIRV::OpVariable)) - .addDef(VarReg) - .addUse(GR.getSPIRVTypeID(VarTy)) - .addImm(SPIRV::StorageClass::UniformConstant) - .addUse(Const); - Result &= MIBVar.constrainAllUses(TII, TRI, RBI); - - GR.add(GV, MIBVar); - GR.addGlobalObject(GV, GR.CurMF, VarReg); - - buildOpDecorate(VarReg, I, TII, SPIRV::Decoration::Constant, {}); - SPIRVType *SourceTy = GR.getOrCreateSPIRVPointerType( - ValTy, I, SPIRV::StorageClass::UniformConstant); - SrcReg = MRI->createGenericVirtualRegister(LLT::scalar(64)); - selectOpWithSrcs(SrcReg, SourceTy, I, {VarReg}, SPIRV::OpBitcast); + addMemoryOperands(*I.memoperands_begin(), MIB, MIRBuilder, GR); } + return MIB.constrainAllUses(TII, TRI, RBI); +} + +bool SPIRVInstructionSelector::selectCopyMemorySized(MachineInstr &I, + Register SrcReg) const { + MachineBasicBlock &BB = *I.getParent(); auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpCopyMemorySized)) .addUse(I.getOperand(0).getReg()) .addUse(SrcReg) @@ -1487,9 +1516,30 @@ bool SPIRVInstructionSelector::selectMemOperation(Register ResVReg, MachineIRBuilder MIRBuilder(I); addMemoryOperands(*I.memoperands_begin(), MIB, MIRBuilder, GR); } - Result &= MIB.constrainAllUses(TII, TRI, RBI); - if (ResVReg.isValid() && ResVReg != MIB->getOperand(0).getReg()) - Result &= BuildCOPY(ResVReg, MIB->getOperand(0).getReg(), I); + return MIB.constrainAllUses(TII, TRI, RBI); +} + +bool SPIRVInstructionSelector::selectMemOperation(Register ResVReg, + MachineInstr &I) const { + Register SrcReg = I.getOperand(1).getReg(); + bool Result = true; + if (I.getOpcode() == TargetOpcode::G_MEMSET) { + Register VarReg = getOrCreateMemSetGlobal(I); + if (!VarReg.isValid()) + return false; + Type *ValTy = Type::getInt8Ty(I.getMF()->getFunction().getContext()); + SPIRVType *SourceTy = GR.getOrCreateSPIRVPointerType( + ValTy, I, SPIRV::StorageClass::UniformConstant); + SrcReg = MRI->createGenericVirtualRegister(LLT::scalar(64)); + Result &= selectOpWithSrcs(SrcReg, SourceTy, I, {VarReg}, SPIRV::OpBitcast); + } + if (STI.isLogicalSPIRV()) { + Result &= selectCopyMemory(I, SrcReg); + } else { + Result &= selectCopyMemorySized(I, SrcReg); + } + if (ResVReg.isValid() && ResVReg != I.getOperand(0).getReg()) + Result &= BuildCOPY(ResVReg, I.getOperand(0).getReg(), I); return Result; } diff --git a/llvm/test/CodeGen/SPIRV/llvm-intrinsics/logical-memcpy.ll b/llvm/test/CodeGen/SPIRV/llvm-intrinsics/logical-memcpy.ll new file mode 100644 index 0000000000000..63eddd20bfc22 --- /dev/null +++ b/llvm/test/CodeGen/SPIRV/llvm-intrinsics/logical-memcpy.ll @@ -0,0 +1,32 @@ +; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s +; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-unknown %s -o - -filetype=obj | spirv-val %} + +; CHECK: OpName %[[dst_var:[0-9]+]] "dst" +; CHECK: OpName %[[src_var:[0-9]+]] "src" + +; CHECK: %[[f32:[0-9]+]] = OpTypeFloat 32 +; CHECK: %[[structS:[0-9]+]] = OpTypeStruct %[[f32]] %[[f32]] %[[f32]] %[[f32]] %[[f32]] +; CHECK: %[[ptr_crosswkgrp_structS:[0-9]+]] = OpTypePointer CrossWorkgroup %[[structS]] +%struct.S = type <{ float, float, float, float, float }> + +; CHECK-DAG: %[[src_var]] = OpVariable %[[ptr_crosswkgrp_structS]] CrossWorkgroup  +@src = external dso_local addrspace(1) global %struct.S, align 4 + +; CHECK-DAG: %[[dst_var]] = OpVariable %[[ptr_crosswkgrp_structS]] CrossWorkgroup  +@dst = external dso_local addrspace(1) global %struct.S, align 4 + +; CHECK: %[[main_func:[0-9]+]] = OpFunction %{{[0-9]+}} None %{{[0-9]+}} +; CHECK: %[[entry:[0-9]+]] = OpLabel +; Function Attrs: mustprogress nofree noinline norecurse nosync nounwind willreturn memory(readwrite, inaccessiblemem: none, target_mem0: none, target_mem1: none) +define void @main() local_unnamed_addr #0 { +entry: +; CHECK: OpCopyMemory %[[dst_var]] %[[src_var]] Aligned 4 + call void @llvm.memcpy.p0.p0.i64(ptr addrspace(1) align 4 @dst, ptr addrspace(1) align 4 @src, i64 20, i1 false) + ret void +; CHECK: OpReturn +; CHECK: OpFunctionEnd +} + +attributes #0 = { "hlsl.numthreads"="8,1,1" "hlsl.shader"="compute" } + + 

[Keenuts]

just a small nit, otherwise LGTM, thanks!