[llvm-branch-commits] [DirectX] Lower `@llvm.dx.typedBufferLoad` to DXIL ops (PR #104252)
https://github.com/dmpots edited https://github.com/llvm/llvm-project/pull/104252 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.typedBufferLoad` to DXIL ops (PR #104252)
https://github.com/dmpots approved this pull request. Overall, LGTM https://github.com/llvm/llvm-project/pull/104252 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.typedBufferLoad` to DXIL ops (PR #104252)
@@ -0,0 +1,102 @@ +; RUN: opt -S -dxil-op-lower %s | FileCheck %s + +target triple = "dxil-pc-shadermodel6.6-compute" + +declare void @scalar_user(float) +declare void @vector_user(<4 x float>) + +define void @loadfloats() { + ; CHECK: [[BIND:%.*]] = call %dx.types.Handle @dx.op.createHandleFromBinding + ; CHECK: [[HANDLE:%.*]] = call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BIND]] + %buffer = call target("dx.TypedBuffer", <4 x float>, 0, 0, 0) dmpots wrote: Is it worth adding a test for a RWBuffer as well? I don't think it changes based on the buffer type, but thought I'd ask. https://github.com/llvm/llvm-project/pull/104252 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.typedBufferLoad` to DXIL ops (PR #104252)
@@ -683,6 +686,17 @@ def CreateHandle : DXILOp<57, createHandle> { let stages = [Stages]; } +def BufferLoad : DXILOp<68, bufferLoad> { + let Doc = "reads from a TypedBuffer"; + // Handle, Coord0, Coord1 + let arguments = [HandleTy, Int32Ty, Int32Ty]; + let result = OverloadTy; + let overloads = + [Overloads]; dmpots wrote: Are the 16-bit overloads valid in dxil 1.0? I suppose maybe they are used to represent ther the minprec types, but true 16-bit types only came in with dxil 1.2 I think. https://github.com/llvm/llvm-project/pull/104252 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.typedBufferLoad` to DXIL ops (PR #104252)
@@ -0,0 +1,102 @@ +; RUN: opt -S -dxil-op-lower %s | FileCheck %s + +target triple = "dxil-pc-shadermodel6.6-compute" + +declare void @scalar_user(float) +declare void @vector_user(<4 x float>) + +define void @loadfloats() { + ; CHECK: [[BIND:%.*]] = call %dx.types.Handle @dx.op.createHandleFromBinding + ; CHECK: [[HANDLE:%.*]] = call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BIND]] + %buffer = call target("dx.TypedBuffer", <4 x float>, 0, 0, 0) + @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_0_0_0( + i32 0, i32 0, i32 1, i32 0, i1 false) + + ; The temporary casts should all have been cleaned up + ; CHECK-NOT: %dx.cast_handle + + ; CHECK: [[DATA0:%.*]] = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(i32 68, %dx.types.Handle [[HANDLE]], i32 0, i32 undef) + %data0 = call <4 x float> @llvm.dx.typedBufferLoad( + target("dx.TypedBuffer", <4 x float>, 0, 0, 0) %buffer, i32 0) + + ; The extract order depends on the users, so don't enforce that here. + ; CHECK-DAG: extractvalue %dx.types.ResRet.f32 [[DATA0]], 0 + %data0_0 = extractelement <4 x float> %data0, i32 0 + ; CHECK-DAG: extractvalue %dx.types.ResRet.f32 [[DATA0]], 2 + %data0_2 = extractelement <4 x float> %data0, i32 2 + + ; If all of the uses are extracts, we skip creating a vector + ; CHECK-NOT: insertelement + call void @scalar_user(float %data0_0) + call void @scalar_user(float %data0_2) + + ; CHECK: [[DATA4:%.*]] = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(i32 68, %dx.types.Handle [[HANDLE]], i32 4, i32 undef) + %data4 = call <4 x float> @llvm.dx.typedBufferLoad( + target("dx.TypedBuffer", <4 x float>, 0, 0, 0) %buffer, i32 4) + + ; CHECK: extractvalue %dx.types.ResRet.f32 [[DATA4]], 0 + ; CHECK: extractvalue %dx.types.ResRet.f32 [[DATA4]], 1 + ; CHECK: extractvalue %dx.types.ResRet.f32 [[DATA4]], 2 + ; CHECK: extractvalue %dx.types.ResRet.f32 [[DATA4]], 3 + ; CHECK: insertelement <4 x float> undef + ; CHECK: insertelement <4 x float> + ; CHECK: insertelement <4 x float> + ; CHECK: insertelement <4 x float> + call void @vector_user(<4 x float> %data4) + + ; CHECK: [[DATA12:%.*]] = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(i32 68, %dx.types.Handle [[HANDLE]], i32 12, i32 undef) + %data12 = call <4 x float> @llvm.dx.typedBufferLoad( + target("dx.TypedBuffer", <4 x float>, 0, 0, 0) %buffer, i32 12) + + ; CHECK: [[DATA12_3:%.*]] = extractvalue %dx.types.ResRet.f32 [[DATA12]], 3 + %data12_3 = extractelement <4 x float> %data12, i32 3 dmpots wrote: It looks like all the `extractelement`s have immediate indices. Probably worth adding a non-immediate index. https://github.com/llvm/llvm-project/pull/104252 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.typedBufferLoad` to DXIL ops (PR #104252)
@@ -236,6 +236,59 @@ class OpLowerer { lowerToBindAndAnnotateHandle(F); } + void lowerTypedBufferLoad(Function &F) { +IRBuilder<> &IRB = OpBuilder.getIRB(); +Type *Int32Ty = IRB.getInt32Ty(); + +replaceFunction(F, [&](CallInst *CI) -> Error { + IRB.SetInsertPoint(CI); + + Value *Handle = + createTmpHandleCast(CI->getArgOperand(0), OpBuilder.getHandleType()); + Value *Index0 = CI->getArgOperand(1); + Value *Index1 = UndefValue::get(Int32Ty); + Type *RetTy = OpBuilder.getResRetType(CI->getType()->getScalarType()); + + std::array Args{Handle, Index0, Index1}; + Expected OpCall = + OpBuilder.tryCreateOp(OpCode::BufferLoad, Args, RetTy); + if (Error E = OpCall.takeError()) +return E; + + std::array Extracts = {}; + + // We've switched the return type from a vector to a struct, but at this + // point most vectors have probably already been scalarized. Try to + // forward arguments directly rather than inserting into and immediately + // extracting from a vector. + for (Use &U : make_early_inc_range(CI->uses())) +if (auto *EEI = dyn_cast(U.getUser())) + if (auto *Index = dyn_cast(EEI->getIndexOperand())) { +size_t IndexVal = Index->getZExtValue(); +assert(IndexVal < 4 && "Index into buffer load out of range"); dmpots wrote: Have we thought about how `CheckAccessFullyMapped` is going to fit into this scheme? In dxil, that will lower to an extract of index 4 on the `dx.types.ResRet` struct. https://github.com/llvm/llvm-project/pull/104252 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.typedBufferLoad` to DXIL ops (PR #104252)
@@ -0,0 +1,102 @@ +; RUN: opt -S -dxil-op-lower %s | FileCheck %s + +target triple = "dxil-pc-shadermodel6.6-compute" + +declare void @scalar_user(float) +declare void @vector_user(<4 x float>) + +define void @loadfloats() { + ; CHECK: [[BIND:%.*]] = call %dx.types.Handle @dx.op.createHandleFromBinding + ; CHECK: [[HANDLE:%.*]] = call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BIND]] + %buffer = call target("dx.TypedBuffer", <4 x float>, 0, 0, 0) + @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_0_0_0( + i32 0, i32 0, i32 1, i32 0, i1 false) + + ; The temporary casts should all have been cleaned up + ; CHECK-NOT: %dx.cast_handle + + ; CHECK: [[DATA0:%.*]] = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(i32 68, %dx.types.Handle [[HANDLE]], i32 0, i32 undef) + %data0 = call <4 x float> @llvm.dx.typedBufferLoad( + target("dx.TypedBuffer", <4 x float>, 0, 0, 0) %buffer, i32 0) + + ; The extract order depends on the users, so don't enforce that here. + ; CHECK-DAG: extractvalue %dx.types.ResRet.f32 [[DATA0]], 0 + %data0_0 = extractelement <4 x float> %data0, i32 0 + ; CHECK-DAG: extractvalue %dx.types.ResRet.f32 [[DATA0]], 2 + %data0_2 = extractelement <4 x float> %data0, i32 2 + + ; If all of the uses are extracts, we skip creating a vector + ; CHECK-NOT: insertelement + call void @scalar_user(float %data0_0) + call void @scalar_user(float %data0_2) + + ; CHECK: [[DATA4:%.*]] = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(i32 68, %dx.types.Handle [[HANDLE]], i32 4, i32 undef) + %data4 = call <4 x float> @llvm.dx.typedBufferLoad( + target("dx.TypedBuffer", <4 x float>, 0, 0, 0) %buffer, i32 4) + + ; CHECK: extractvalue %dx.types.ResRet.f32 [[DATA4]], 0 + ; CHECK: extractvalue %dx.types.ResRet.f32 [[DATA4]], 1 + ; CHECK: extractvalue %dx.types.ResRet.f32 [[DATA4]], 2 + ; CHECK: extractvalue %dx.types.ResRet.f32 [[DATA4]], 3 + ; CHECK: insertelement <4 x float> undef + ; CHECK: insertelement <4 x float> + ; CHECK: insertelement <4 x float> + ; CHECK: insertelement <4 x float> + call void @vector_user(<4 x float> %data4) dmpots wrote: I understand why you use a call here, but this is going to produce something that we cannot actually generate valid dxil for right? Should we have tests where we produce invalid dxil? Maybe this is fine here because it is only lowering ops and we have some dxil validation later? https://github.com/llvm/llvm-project/pull/104252 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.typedBufferLoad` to DXIL ops (PR #104252)
@@ -0,0 +1,102 @@ +; RUN: opt -S -dxil-op-lower %s | FileCheck %s + +target triple = "dxil-pc-shadermodel6.6-compute" + +declare void @scalar_user(float) +declare void @vector_user(<4 x float>) + +define void @loadfloats() { + ; CHECK: [[BIND:%.*]] = call %dx.types.Handle @dx.op.createHandleFromBinding + ; CHECK: [[HANDLE:%.*]] = call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BIND]] + %buffer = call target("dx.TypedBuffer", <4 x float>, 0, 0, 0) + @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_0_0_0( + i32 0, i32 0, i32 1, i32 0, i1 false) + + ; The temporary casts should all have been cleaned up + ; CHECK-NOT: %dx.cast_handle + + ; CHECK: [[DATA0:%.*]] = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(i32 68, %dx.types.Handle [[HANDLE]], i32 0, i32 undef) + %data0 = call <4 x float> @llvm.dx.typedBufferLoad( + target("dx.TypedBuffer", <4 x float>, 0, 0, 0) %buffer, i32 0) + + ; The extract order depends on the users, so don't enforce that here. + ; CHECK-DAG: extractvalue %dx.types.ResRet.f32 [[DATA0]], 0 + %data0_0 = extractelement <4 x float> %data0, i32 0 + ; CHECK-DAG: extractvalue %dx.types.ResRet.f32 [[DATA0]], 2 + %data0_2 = extractelement <4 x float> %data0, i32 2 + + ; If all of the uses are extracts, we skip creating a vector + ; CHECK-NOT: insertelement + call void @scalar_user(float %data0_0) + call void @scalar_user(float %data0_2) + + ; CHECK: [[DATA4:%.*]] = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(i32 68, %dx.types.Handle [[HANDLE]], i32 4, i32 undef) + %data4 = call <4 x float> @llvm.dx.typedBufferLoad( + target("dx.TypedBuffer", <4 x float>, 0, 0, 0) %buffer, i32 4) + + ; CHECK: extractvalue %dx.types.ResRet.f32 [[DATA4]], 0 + ; CHECK: extractvalue %dx.types.ResRet.f32 [[DATA4]], 1 + ; CHECK: extractvalue %dx.types.ResRet.f32 [[DATA4]], 2 + ; CHECK: extractvalue %dx.types.ResRet.f32 [[DATA4]], 3 + ; CHECK: insertelement <4 x float> undef + ; CHECK: insertelement <4 x float> + ; CHECK: insertelement <4 x float> + ; CHECK: insertelement <4 x float> + call void @vector_user(<4 x float> %data4) + + ; CHECK: [[DATA12:%.*]] = call %dx.types.ResRet.f32 @dx.op.bufferLoad.f32(i32 68, %dx.types.Handle [[HANDLE]], i32 12, i32 undef) + %data12 = call <4 x float> @llvm.dx.typedBufferLoad( dmpots wrote: Does the `llvm.dx.typedBufferLoad` intrinsic always return a vec4? Wondering about a `Buffer` would generate a different overload of the intrinsic call. https://github.com/llvm/llvm-project/pull/104252 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [HLSL] Add helpers to simplify HLSL resource type declarations. NFC (PR #73967)
https://github.com/dmpots approved this pull request. https://github.com/llvm/llvm-project/pull/73967 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL] Define RasterizerOrderedBuffer resource (PR #74897)
dmpots wrote: This PR seems to be missing a description. https://github.com/llvm/llvm-project/pull/74897 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL] Define RasterizerOrderedBuffer resource (PR #74897)
https://github.com/dmpots approved this pull request. With a description and comment in the test it otherwise LGTM. https://github.com/llvm/llvm-project/pull/74897 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL] Define RasterizerOrderedBuffer resource (PR #74897)
https://github.com/dmpots edited https://github.com/llvm/llvm-project/pull/74897 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [clang] [llvm] [HLSL] Define RasterizerOrderedBuffer resource (PR #74897)
@@ -0,0 +1,20 @@ +// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s dmpots wrote: I think this is fine for testing this change, but ROVs are not allowed to be used in CS (only PS). This shader does compile successfully because they buffers are not used, but a use here will cause a validation error https://godbolt.org/z/zEWr7bdsP ``` error: validation errors error: RasterizerOrdered objects are only allowed in 5.0+ pixel shaders. 'Buffer1' Validation failed. ``` Probably worth a comment in the test for this. https://github.com/llvm/llvm-project/pull/74897 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.handle.fromBinding` to DXIL ops (PR #104251)
@@ -683,6 +685,14 @@ def Dot4 : DXILOp<56, dot4> { let attributes = [Attributes]; } +def CreateHandle : DXILOp<57, createHandle> { + let Doc = "creates the handle to a resource"; + // ResourceClass, RangeID, Index, NonUniform + let arguments = [Int8Ty, Int32Ty, Int32Ty, Int1Ty]; + let result = HandleTy; + let stages = [Stages]; dmpots wrote: This should be invalid starting in DXIL_1_6 I think, right? Did we add a way to express that in the TD definition? https://github.com/llvm/llvm-project/pull/104251 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.handle.fromBinding` to DXIL ops (PR #104251)
@@ -119,6 +123,119 @@ class OpLowerer { }); } + Value *createTmpHandleCast(Value *V, Type *Ty) { +Function *CastFn = Intrinsic::getDeclaration(&M, Intrinsic::dx_cast_handle, + {Ty, V->getType()}); +CallInst *Cast = OpBuilder.getIRB().CreateCall(CastFn, {V}); +CleanupCasts.push_back(Cast); +return Cast; + } + + void cleanupHandleCasts() { +SmallVector ToRemove; +SmallVector CastFns; + +for (CallInst *Cast : CleanupCasts) { + CastFns.push_back(Cast->getCalledFunction()); + // All of the ops should be using `dx.types.Handle` at this point, so if + // we're not producing that we should be part of a pair. Track this so we dmpots wrote: It's not clear from reading what "it should be part of a pair" means and why it must be true. Can we expand the comment here to explain? Is there an assert we should add here as well? https://github.com/llvm/llvm-project/pull/104251 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.handle.fromBinding` to DXIL ops (PR #104251)
@@ -119,6 +123,119 @@ class OpLowerer { }); } + Value *createTmpHandleCast(Value *V, Type *Ty) { +Function *CastFn = Intrinsic::getDeclaration(&M, Intrinsic::dx_cast_handle, + {Ty, V->getType()}); +CallInst *Cast = OpBuilder.getIRB().CreateCall(CastFn, {V}); +CleanupCasts.push_back(Cast); +return Cast; + } + + void cleanupHandleCasts() { +SmallVector ToRemove; +SmallVector CastFns; + +for (CallInst *Cast : CleanupCasts) { + CastFns.push_back(Cast->getCalledFunction()); + // All of the ops should be using `dx.types.Handle` at this point, so if + // we're not producing that we should be part of a pair. Track this so we + // can remove it at the end. + if (Cast->getType() != OpBuilder.getHandleType()) { +ToRemove.push_back(Cast); +continue; + } + // Otherwise, we're the second handle in a pair. Forward the arguments and + // remove the (second) cast. + CallInst *Def = cast(Cast->getOperand(0)); + assert(Def->getIntrinsicID() == Intrinsic::dx_cast_handle && + "Unbalanced pair of temporary handle casts"); + Cast->replaceAllUsesWith(Def->getOperand(0)); + Cast->eraseFromParent(); +} +for (CallInst *Cast : ToRemove) { + assert(Cast->user_empty() && "Temporary handle cast still has users"); + Cast->eraseFromParent(); +} +llvm::sort(CastFns); +CastFns.erase(llvm::unique(CastFns), CastFns.end()); +for (Function *F : CastFns) + F->eraseFromParent(); + +CleanupCasts.clear(); + } + + void lowerToCreateHandle(Function &F) { +IRBuilder<> &IRB = OpBuilder.getIRB(); +Type *Int8Ty = IRB.getInt8Ty(); +Type *Int32Ty = IRB.getInt32Ty(); + +replaceFunction(F, [&](CallInst *CI) -> Error { + IRB.SetInsertPoint(CI); + + dxil::ResourceInfo &RI = DRM[CI]; + dxil::ResourceInfo::ResourceBinding Binding = RI.getBinding(); + + std::array Args{ + ConstantInt::get(Int8Ty, llvm::to_underlying(RI.getResourceClass())), + ConstantInt::get(Int32Ty, Binding.RecordID), CI->getArgOperand(3), + CI->getArgOperand(4)}; + Expected OpCall = + OpBuilder.tryCreateOp(OpCode::CreateHandle, Args); + if (Error E = OpCall.takeError()) +return E; + + Value *Cast = createTmpHandleCast(*OpCall, CI->getType()); + + CI->replaceAllUsesWith(Cast); + CI->eraseFromParent(); + return Error::success(); +}); + } + + void lowerToBindAndAnnotateHandle(Function &F) { +IRBuilder<> &IRB = OpBuilder.getIRB(); + +replaceFunction(F, [&](CallInst *CI) -> Error { + IRB.SetInsertPoint(CI); + + dxil::ResourceInfo &RI = DRM[CI]; + dxil::ResourceInfo::ResourceBinding Binding = RI.getBinding(); + std::pair Props = RI.getAnnotateProps(); + + Constant *ResBind = OpBuilder.getResBind( + Binding.LowerBound, Binding.LowerBound + Binding.Size - 1, dmpots wrote: Is this going to do the right thing for unbounded resource array size? I think that should have an upper bound of UINT_MAX. https://github.com/llvm/llvm-project/pull/104251 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.handle.fromBinding` to DXIL ops (PR #104251)
@@ -119,6 +123,119 @@ class OpLowerer { }); } + Value *createTmpHandleCast(Value *V, Type *Ty) { +Function *CastFn = Intrinsic::getDeclaration(&M, Intrinsic::dx_cast_handle, + {Ty, V->getType()}); +CallInst *Cast = OpBuilder.getIRB().CreateCall(CastFn, {V}); +CleanupCasts.push_back(Cast); +return Cast; + } + + void cleanupHandleCasts() { +SmallVector ToRemove; +SmallVector CastFns; + +for (CallInst *Cast : CleanupCasts) { + CastFns.push_back(Cast->getCalledFunction()); + // All of the ops should be using `dx.types.Handle` at this point, so if + // we're not producing that we should be part of a pair. Track this so we + // can remove it at the end. + if (Cast->getType() != OpBuilder.getHandleType()) { +ToRemove.push_back(Cast); +continue; + } + // Otherwise, we're the second handle in a pair. Forward the arguments and + // remove the (second) cast. + CallInst *Def = cast(Cast->getOperand(0)); + assert(Def->getIntrinsicID() == Intrinsic::dx_cast_handle && + "Unbalanced pair of temporary handle casts"); + Cast->replaceAllUsesWith(Def->getOperand(0)); + Cast->eraseFromParent(); +} +for (CallInst *Cast : ToRemove) { + assert(Cast->user_empty() && "Temporary handle cast still has users"); + Cast->eraseFromParent(); +} +llvm::sort(CastFns); +CastFns.erase(llvm::unique(CastFns), CastFns.end()); +for (Function *F : CastFns) + F->eraseFromParent(); + +CleanupCasts.clear(); + } + + void lowerToCreateHandle(Function &F) { +IRBuilder<> &IRB = OpBuilder.getIRB(); +Type *Int8Ty = IRB.getInt8Ty(); +Type *Int32Ty = IRB.getInt32Ty(); + +replaceFunction(F, [&](CallInst *CI) -> Error { + IRB.SetInsertPoint(CI); + + dxil::ResourceInfo &RI = DRM[CI]; + dxil::ResourceInfo::ResourceBinding Binding = RI.getBinding(); + + std::array Args{ + ConstantInt::get(Int8Ty, llvm::to_underlying(RI.getResourceClass())), + ConstantInt::get(Int32Ty, Binding.RecordID), CI->getArgOperand(3), + CI->getArgOperand(4)}; + Expected OpCall = + OpBuilder.tryCreateOp(OpCode::CreateHandle, Args); + if (Error E = OpCall.takeError()) +return E; + + Value *Cast = createTmpHandleCast(*OpCall, CI->getType()); + + CI->replaceAllUsesWith(Cast); + CI->eraseFromParent(); + return Error::success(); +}); + } + + void lowerToBindAndAnnotateHandle(Function &F) { +IRBuilder<> &IRB = OpBuilder.getIRB(); + +replaceFunction(F, [&](CallInst *CI) -> Error { + IRB.SetInsertPoint(CI); + + dxil::ResourceInfo &RI = DRM[CI]; + dxil::ResourceInfo::ResourceBinding Binding = RI.getBinding(); + std::pair Props = RI.getAnnotateProps(); + + Constant *ResBind = OpBuilder.getResBind( + Binding.LowerBound, Binding.LowerBound + Binding.Size - 1, + Binding.Space, RI.getResourceClass()); + std::array BindArgs{ResBind, CI->getArgOperand(3), + CI->getArgOperand(4)}; + Expected OpBind = + OpBuilder.tryCreateOp(OpCode::CreateHandleFromBinding, BindArgs); + if (Error E = OpBind.takeError()) +return E; + + std::array AnnotateArgs{ + *OpBind, OpBuilder.getResProps(Props.first, Props.second)}; + Expected OpAnnotate = + OpBuilder.tryCreateOp(OpCode::AnnotateHandle, AnnotateArgs); + if (Error E = OpAnnotate.takeError()) +return E; + + Value *Cast = createTmpHandleCast(*OpAnnotate, CI->getType()); + + CI->replaceAllUsesWith(Cast); + CI->eraseFromParent(); + + return Error::success(); +}); + } + + void lowerHandleFromBinding(Function &F) { dmpots wrote: This seems to be a more complicated lowering that a straightforward translation (well not this function but its implementation details). Can we add a high-level description of what the lowering does? Like the need for and usage of `dx_cast_handle` would be good to explain. https://github.com/llvm/llvm-project/pull/104251 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.handle.fromBinding` to DXIL ops (PR #104251)
@@ -0,0 +1,61 @@ +; RUN: opt -S -dxil-op-lower %s | FileCheck %s dmpots wrote: I don't see tests for either 1. Unbounded resource arrays 2. Non-constant index into resource arrays I think it would be good to have tests for these. https://github.com/llvm/llvm-project/pull/104251 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.handle.fromBinding` to DXIL ops (PR #104251)
@@ -119,6 +123,119 @@ class OpLowerer { }); } + Value *createTmpHandleCast(Value *V, Type *Ty) { +Function *CastFn = Intrinsic::getDeclaration(&M, Intrinsic::dx_cast_handle, + {Ty, V->getType()}); +CallInst *Cast = OpBuilder.getIRB().CreateCall(CastFn, {V}); +CleanupCasts.push_back(Cast); +return Cast; + } + + void cleanupHandleCasts() { +SmallVector ToRemove; +SmallVector CastFns; + +for (CallInst *Cast : CleanupCasts) { + CastFns.push_back(Cast->getCalledFunction()); + // All of the ops should be using `dx.types.Handle` at this point, so if + // we're not producing that we should be part of a pair. Track this so we + // can remove it at the end. + if (Cast->getType() != OpBuilder.getHandleType()) { +ToRemove.push_back(Cast); +continue; + } + // Otherwise, we're the second handle in a pair. Forward the arguments and + // remove the (second) cast. + CallInst *Def = cast(Cast->getOperand(0)); + assert(Def->getIntrinsicID() == Intrinsic::dx_cast_handle && + "Unbalanced pair of temporary handle casts"); + Cast->replaceAllUsesWith(Def->getOperand(0)); + Cast->eraseFromParent(); +} +for (CallInst *Cast : ToRemove) { + assert(Cast->user_empty() && "Temporary handle cast still has users"); + Cast->eraseFromParent(); +} +llvm::sort(CastFns); +CastFns.erase(llvm::unique(CastFns), CastFns.end()); +for (Function *F : CastFns) + F->eraseFromParent(); dmpots wrote: The explanation is good, can we get that added as a comment? https://github.com/llvm/llvm-project/pull/104251 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.handle.fromBinding` to DXIL ops (PR #104251)
https://github.com/dmpots approved this pull request. Overall, LGTM! https://github.com/llvm/llvm-project/pull/104251 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.handle.fromBinding` to DXIL ops (PR #104251)
@@ -0,0 +1,58 @@ +; RUN: opt -S -dxil-op-lower %s | FileCheck %s + +target triple = "dxil-pc-shadermodel6.6-compute" + +declare i32 @some_val(); + +define void @test_bindings() { + ; RWBuffer Buf : register(u5, space3) + %typed0 = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0) + @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_1_0_0( + i32 3, i32 5, i32 1, i32 4, i1 false) + ; CHECK: [[BUF0:%[0-9]*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 5, i32 5, i32 3, i8 1 }, i32 4, i1 false) + ; CHECK: call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BUF0]], %dx.types.ResourceProperties { i32 4106, i32 1033 }) + + ; RWBuffer Buf : register(u7, space2) + %typed1 = call target("dx.TypedBuffer", i32, 1, 0, 1) + @llvm.dx.handle.fromBinding.tdx.TypedBuffer_i32_1_0_0t( + i32 2, i32 7, i32 1, i32 6, i1 false) + ; CHECK: [[BUF1:%[0-9]*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 7, i32 7, i32 2, i8 1 }, i32 6, i1 false) + ; CHECK: call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BUF1]], %dx.types.ResourceProperties { i32 4106, i32 260 }) + + ; Buffer Buf[24] : register(t3, space5) + ; Buffer typed2 = Buf[4] + ; Note that the index below is 3 + 4 = 7 + %typed2 = call target("dx.TypedBuffer", <4 x i32>, 0, 0, 0) + @llvm.dx.handle.fromBinding.tdx.TypedBuffer_i32_0_0_0t( + i32 5, i32 3, i32 24, i32 7, i1 false) + ; CHECK: [[BUF2:%[0-9]*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 3, i32 26, i32 5, i8 0 }, i32 7, i1 false) + ; CHECK: call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BUF2]], %dx.types.ResourceProperties { i32 10, i32 1029 }) + + ; struct S { float4 a; uint4 b; }; + ; StructuredBuffer Buf : register(t2, space4) + %struct0 = call target("dx.RawBuffer", {<4 x float>, <4 x i32>}, 0, 0) + @llvm.dx.handle.fromBinding.tdx.RawBuffer_sl_v4f32v4i32s_0_0t( + i32 4, i32 2, i32 1, i32 10, i1 true) + ; CHECK: [[BUF3:%[0-9]*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 2, i32 2, i32 4, i8 0 }, i32 10, i1 true) + ; CHECK: = call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BUF3]], %dx.types.ResourceProperties { i32 1036, i32 32 }) + + ; ByteAddressBuffer Buf : register(t8, space1) + %byteaddr0 = call target("dx.RawBuffer", i8, 0, 0) + @llvm.dx.handle.fromBinding.tdx.RawBuffer_i8_0_0t( + i32 1, i32 8, i32 1, i32 12, i1 false) + ; CHECK: [[BUF4:%[0-9]*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 8, i32 8, i32 1, i8 0 }, i32 12, i1 false) + ; CHECK: call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BUF4]], %dx.types.ResourceProperties { i32 11, i32 0 }) + + ; Buffer Buf[] : register(t0) + ; Buffer typed3 = Buf[ix] + %typed3_ix = call i32 @some_val() + %typed3 = call target("dx.TypedBuffer", <4 x float>, 0, 0, 0) + @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_0_0_0t( + i32 0, i32 0, i32 -1, i32 %typed3_ix, i1 false) dmpots wrote: Is it worth adding a test for a non-constant index into a bounded resource range? ``` Buffer Buf[24]; Buffer typed = Buf[ix]; ``` https://github.com/llvm/llvm-project/pull/104251 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.handle.fromBinding` to DXIL ops (PR #104251)
@@ -119,6 +123,142 @@ class OpLowerer { }); } + /// Create a cast between a `target("dx")` type and `dx.types.Handle`, which + /// is intended to be removed by the end of lowering. This is used to allow + /// lowering of ops which need to change their return or argument types in a + /// piecemeal way - we can add the casts in to avoid updating all of the uses + /// or defs, and by the end all of the casts will be redundant. + Value *createTmpHandleCast(Value *V, Type *Ty) { +Function *CastFn = Intrinsic::getDeclaration(&M, Intrinsic::dx_cast_handle, + {Ty, V->getType()}); +CallInst *Cast = OpBuilder.getIRB().CreateCall(CastFn, {V}); +CleanupCasts.push_back(Cast); +return Cast; + } + + void cleanupHandleCasts() { +SmallVector ToRemove; +SmallVector CastFns; + +for (CallInst *Cast : CleanupCasts) { + // These casts were only put in to ease the move from `target("dx")` types + // to `dx.types.Handle in a piecemeal way. At this point, all of the + // non-cast uses should now be `dx.types.Handle`, and remaining casts + // should all form pairs to and from the now unused `target("dx")` type. + CastFns.push_back(Cast->getCalledFunction()); + + // If the cast is not to `dx.types.Handle`, it should be the first part of + // the pair. Keep track so we can remove it once it has no more uses. + if (Cast->getType() != OpBuilder.getHandleType()) { +ToRemove.push_back(Cast); +continue; + } + // Otherwise, we're the second handle in a pair. Forward the arguments and + // remove the (second) cast. + CallInst *Def = cast(Cast->getOperand(0)); + assert(Def->getIntrinsicID() == Intrinsic::dx_cast_handle && + "Unbalanced pair of temporary handle casts"); + Cast->replaceAllUsesWith(Def->getOperand(0)); + Cast->eraseFromParent(); +} +for (CallInst *Cast : ToRemove) { + assert(Cast->user_empty() && "Temporary handle cast still has users"); + Cast->eraseFromParent(); +} + +// Deduplicate the cast functions so that we only erase each one once. +llvm::sort(CastFns); +CastFns.erase(llvm::unique(CastFns), CastFns.end()); +for (Function *F : CastFns) + F->eraseFromParent(); + +CleanupCasts.clear(); + } + + void lowerToCreateHandle(Function &F) { +IRBuilder<> &IRB = OpBuilder.getIRB(); +Type *Int8Ty = IRB.getInt8Ty(); +Type *Int32Ty = IRB.getInt32Ty(); + +replaceFunction(F, [&](CallInst *CI) -> Error { + IRB.SetInsertPoint(CI); + + auto *It = DRM.find(CI); + assert(It != DRM.end() && "Resource not in map?"); + dxil::ResourceInfo &RI = *It; + const auto &Binding = RI.getBinding(); + + std::array Args{ + ConstantInt::get(Int8Ty, llvm::to_underlying(RI.getResourceClass())), + ConstantInt::get(Int32Ty, Binding.RecordID), CI->getArgOperand(3), + CI->getArgOperand(4)}; + Expected OpCall = + OpBuilder.tryCreateOp(OpCode::CreateHandle, Args); + if (Error E = OpCall.takeError()) +return E; + + Value *Cast = createTmpHandleCast(*OpCall, CI->getType()); + + CI->replaceAllUsesWith(Cast); + CI->eraseFromParent(); + return Error::success(); +}); + } + + void lowerToBindAndAnnotateHandle(Function &F) { +IRBuilder<> &IRB = OpBuilder.getIRB(); + +replaceFunction(F, [&](CallInst *CI) -> Error { + IRB.SetInsertPoint(CI); + + auto *It = DRM.find(CI); + assert(It != DRM.end() && "Resource not in map?"); + dxil::ResourceInfo &RI = *It; + + const auto &Binding = RI.getBinding(); + std::pair Props = RI.getAnnotateProps(); + + // For `CreateHandleFromBinding` we need the upper bound rather than the + // size, so we need to be careful about the difference for "unbounded". + uint32_t Unbounded = std::numeric_limits::max(); dmpots wrote: Maybe worth putting in a header somewhere since it is probably not the last use of unbounded that we will have. https://github.com/llvm/llvm-project/pull/104251 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.handle.fromBinding` to DXIL ops (PR #104251)
https://github.com/dmpots edited https://github.com/llvm/llvm-project/pull/104251 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
[llvm-branch-commits] [DirectX] Lower `@llvm.dx.handle.fromBinding` to DXIL ops (PR #104251)
@@ -0,0 +1,53 @@ +; RUN: opt -S -dxil-op-lower %s | FileCheck %s + +target triple = "dxil-pc-shadermodel6.0-compute" + +declare i32 @some_val(); + +define void @test_buffers() { + ; RWBuffer Buf : register(u5, space3) + %typed0 = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0) + @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_1_0_0( + i32 3, i32 5, i32 1, i32 4, i1 false) + ; CHECK: call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 1, i32 4, i1 false) + ; CHECK-NOT: @llvm.dx.cast.handle + + ; RWBuffer Buf : register(u7, space2) + %typed1 = call target("dx.TypedBuffer", i32, 1, 0, 1) + @llvm.dx.handle.fromBinding.tdx.TypedBuffer_i32_1_0_1t( + i32 2, i32 7, i32 1, i32 6, i1 false) + ; CHECK: call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 6, i1 false) + + ; Buffer Buf[24] : register(t3, space5) + ; Buffer typed2 = Buf[5] + ; Note that the index below is 3 + 4 = 7 dmpots wrote: Is this comment out of sync with the line above? Meaning should the line above be `Buf[4]`? I'm not seeing where the 4 comes from otherwise. https://github.com/llvm/llvm-project/pull/104251 ___ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits