From 7cdea51a398a07c7c4463a67968870bee78cca0a Mon Sep 17 00:00:00 2001 From: Justin Bogner Date: Fri, 21 Feb 2025 12:58:55 -0800 Subject: [PATCH 1/5] [DirectX][OpLowering] Simplify named struct handling This removes "replaceFunctionWithNamedStructOp" and folds its functionality into "replaceFunctionWithOp". It turns out we were overcomplicating things and this is trivial to handle generically. Fixes #113192 --- llvm/lib/Target/DirectX/DXIL.td | 1 + llvm/lib/Target/DirectX/DXILOpLowering.cpp | 61 ++++++++++------------ 2 files changed, 28 insertions(+), 34 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXIL.td b/llvm/lib/Target/DirectX/DXIL.td index 7cb841d9bd5b5..d59e28c37b91d 100644 --- a/llvm/lib/Target/DirectX/DXIL.td +++ b/llvm/lib/Target/DirectX/DXIL.td @@ -930,6 +930,7 @@ def MakeDouble : DXILOp<101, makeDouble> { def SplitDouble : DXILOp<102, splitDouble> { let Doc = "Splits a double into 2 uints"; + let intrinsics = [IntrinSelect]; let arguments = [OverloadTy]; let result = SplitDoubleTy; let overloads = [Overloads]; diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp b/llvm/lib/Target/DirectX/DXILOpLowering.cpp index 0c245c1a43d31..5459695f96863 100644 --- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp +++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp @@ -120,6 +120,28 @@ class OpLowerer { int Value; }; + /// Replaces uses of a struct with uses of an equivalent named struct. + /// + /// DXIL operations that return structs give them well known names, so we need + /// to update uses when we switch from an LLVM intrinsic to an op. + Error replaceNamedStructUses(CallInst *Intrin, CallInst *DXILOp) { + auto *IntrinTy = cast(Intrin->getType()); + auto *DXILOpTy = cast(DXILOp->getType()); + if (!IntrinTy->isLayoutIdentical(DXILOpTy)) + return make_error( + "Type mismatch between intrinsic and DXIL op", + inconvertibleErrorCode()); + + for (Use &U : make_early_inc_range(Intrin->uses())) + if (auto *EVI = dyn_cast(U.getUser())) + EVI->setOperand(0, DXILOp); + else + return make_error( + "DXIL ops that return structs may only be used by extractvalue", + inconvertibleErrorCode()); + return Error::success(); + } + [[nodiscard]] bool replaceFunctionWithOp(Function &F, dxil::OpCode DXILOp, ArrayRef ArgSelects) { @@ -154,32 +176,13 @@ class OpLowerer { if (Error E = OpCall.takeError()) return E; - CI->replaceAllUsesWith(*OpCall); - CI->eraseFromParent(); - return Error::success(); - }); - } - - [[nodiscard]] bool replaceFunctionWithNamedStructOp( - Function &F, dxil::OpCode DXILOp, Type *NewRetTy, - llvm::function_ref ReplaceUses) { - bool IsVectorArgExpansion = isVectorArgExpansion(F); - return replaceFunction(F, [&](CallInst *CI) -> Error { - SmallVector Args; - OpBuilder.getIRB().SetInsertPoint(CI); - if (IsVectorArgExpansion) { - SmallVector NewArgs = argVectorFlatten(CI, OpBuilder.getIRB()); - Args.append(NewArgs.begin(), NewArgs.end()); + if (isa(CI->getType())) { + if (Error E = replaceNamedStructUses(CI, *OpCall)) + return E; } else - Args.append(CI->arg_begin(), CI->arg_end()); - - Expected OpCall = - OpBuilder.tryCreateOp(DXILOp, Args, CI->getName(), NewRetTy); - if (Error E = OpCall.takeError()) - return E; - if (Error E = ReplaceUses(CI, *OpCall)) - return E; + CI->replaceAllUsesWith(*OpCall); + CI->eraseFromParent(); return Error::success(); }); } @@ -814,16 +817,6 @@ class OpLowerer { case Intrinsic::dx_resource_updatecounter: HasErrors |= lowerUpdateCounter(F); break; - // TODO: this can be removed when - // https://github.com/llvm/llvm-project/issues/113192 is fixed - case Intrinsic::dx_splitdouble: - HasErrors |= replaceFunctionWithNamedStructOp( - F, OpCode::SplitDouble, - OpBuilder.getSplitDoubleType(M.getContext()), - [&](CallInst *CI, CallInst *Op) { - return replaceSplitDoubleCallUsages(CI, Op); - }); - break; case Intrinsic::ctpop: HasErrors |= lowerCtpopToCountBits(F); break; From 6a7460cffc3505cd176e983f17099f49dc100d36 Mon Sep 17 00:00:00 2001 From: Justin Bogner Date: Fri, 21 Feb 2025 15:36:26 -0800 Subject: [PATCH 2/5] Handle insertvalue --- llvm/lib/Target/DirectX/DXILOpLowering.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp b/llvm/lib/Target/DirectX/DXILOpLowering.cpp index 5459695f96863..d50e2c938edeb 100644 --- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp +++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp @@ -135,10 +135,12 @@ class OpLowerer { for (Use &U : make_early_inc_range(Intrin->uses())) if (auto *EVI = dyn_cast(U.getUser())) EVI->setOperand(0, DXILOp); + else if (auto *IVI = dyn_cast(U.getUser())) + IVI->setOperand(0, DXILOp); else - return make_error( - "DXIL ops that return structs may only be used by extractvalue", - inconvertibleErrorCode()); + return make_error("DXIL ops that return structs may only " + "be used by insert- and extractvalue", + inconvertibleErrorCode()); return Error::success(); } From c3544452d42715a36503449a57f4700efd2fb6a8 Mon Sep 17 00:00:00 2001 From: Justin Bogner Date: Fri, 21 Feb 2025 15:40:11 -0800 Subject: [PATCH 3/5] remove unused function --- llvm/lib/Target/DirectX/DXILOpBuilder.cpp | 3 --- llvm/lib/Target/DirectX/DXILOpBuilder.h | 3 --- 2 files changed, 6 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILOpBuilder.cpp b/llvm/lib/Target/DirectX/DXILOpBuilder.cpp index badd5aabd6432..4c2479bc08b1f 100644 --- a/llvm/lib/Target/DirectX/DXILOpBuilder.cpp +++ b/llvm/lib/Target/DirectX/DXILOpBuilder.cpp @@ -535,9 +535,6 @@ StructType *DXILOpBuilder::getResRetType(Type *ElementTy) { return ::getResRetType(ElementTy); } -StructType *DXILOpBuilder::getSplitDoubleType(LLVMContext &Context) { - return ::getSplitDoubleType(Context); -} StructType *DXILOpBuilder::getHandleType() { return ::getHandleType(IRB.getContext()); diff --git a/llvm/lib/Target/DirectX/DXILOpBuilder.h b/llvm/lib/Target/DirectX/DXILOpBuilder.h index df5a0240870f4..5fe9f4429a494 100644 --- a/llvm/lib/Target/DirectX/DXILOpBuilder.h +++ b/llvm/lib/Target/DirectX/DXILOpBuilder.h @@ -50,9 +50,6 @@ class DXILOpBuilder { /// Get a `%dx.types.ResRet` type with the given element type. StructType *getResRetType(Type *ElementTy); - /// Get the `%dx.types.splitdouble` type. - StructType *getSplitDoubleType(LLVMContext &Context); - /// Get the `%dx.types.Handle` type. StructType *getHandleType(); From af21e2e78a6ce7177e0a48ac4f12a7d500223220 Mon Sep 17 00:00:00 2001 From: Justin Bogner Date: Fri, 21 Feb 2025 15:46:49 -0800 Subject: [PATCH 4/5] clang-format --- llvm/lib/Target/DirectX/DXILOpBuilder.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/llvm/lib/Target/DirectX/DXILOpBuilder.cpp b/llvm/lib/Target/DirectX/DXILOpBuilder.cpp index 4c2479bc08b1f..6bbe8d5d12280 100644 --- a/llvm/lib/Target/DirectX/DXILOpBuilder.cpp +++ b/llvm/lib/Target/DirectX/DXILOpBuilder.cpp @@ -535,7 +535,6 @@ StructType *DXILOpBuilder::getResRetType(Type *ElementTy) { return ::getResRetType(ElementTy); } - StructType *DXILOpBuilder::getHandleType() { return ::getHandleType(IRB.getContext()); } From 0f05e703c026a52682efa733db5fe0f22c5a285c Mon Sep 17 00:00:00 2001 From: Justin Bogner Date: Sat, 22 Feb 2025 11:21:17 -0800 Subject: [PATCH 5/5] remove unused function --- llvm/lib/Target/DirectX/DXILOpLowering.cpp | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp b/llvm/lib/Target/DirectX/DXILOpLowering.cpp index d50e2c938edeb..83cc4b18824c7 100644 --- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp +++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp @@ -364,26 +364,6 @@ class OpLowerer { return lowerToBindAndAnnotateHandle(F); } - Error replaceSplitDoubleCallUsages(CallInst *Intrin, CallInst *Op) { - for (Use &U : make_early_inc_range(Intrin->uses())) { - if (auto *EVI = dyn_cast(U.getUser())) { - - if (EVI->getNumIndices() != 1) - return createStringError(std::errc::invalid_argument, - "Splitdouble has only 2 elements"); - EVI->setOperand(0, Op); - } else { - return make_error( - "Splitdouble use is not ExtractValueInst", - inconvertibleErrorCode()); - } - } - - Intrin->eraseFromParent(); - - return Error::success(); - } - /// Replace uses of \c Intrin with the values in the `dx.ResRet` of \c Op. /// Since we expect to be post-scalarization, make an effort to avoid vectors. Error replaceResRetUses(CallInst *Intrin, CallInst *Op, bool HasCheckBit) {