[DirectX][NFC] Use LLVM Types in DXIL Operation specifications in DXIL.td #81692
Conversation
This file contains 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
This change aims to use a single class abstraction to specify valid overload types of DXIL operation and to specify parameter types of DXIL operation. Updated (a) parameter types accordingly in the specification of existing DXILOperations and (b) DXILEmitter.
|
@llvm/pr-subscribers-backend-directx Author: S. Bharadwaj Yadavalli (bharadwajy) ChangesThis change aims to use a single class abstraction to specify valid overload types of DXIL operation and to specify parameter types of DXIL operation. Updated (a) parameter types accordingly in the specification of existing DXILOperations and (b) DXILEmitter. Full diff: https://github.com/llvm/llvm-project/pull/81692.diff 2 Files Affected:
diff --git a/llvm/lib/Target/DirectX/DXIL.td b/llvm/lib/Target/DirectX/DXIL.td
index 52158139a2584e..a783b2be48dbdd 100644
--- a/llvm/lib/Target/DirectX/DXIL.td
+++ b/llvm/lib/Target/DirectX/DXIL.td
@@ -35,30 +35,78 @@ def BinaryUintCategory : DXILOpCategory<"Binary uint">;
def UnaryFloatCategory : DXILOpCategory<"Unary float">;
def ComputeIDCategory : DXILOpCategory<"Compute/Mesh/Amplification shader">;
-// Following are the scalar types supported by DXIL operations and are synonymous
-// to llvm_*_ty defined for readability and ease of use in the context of this file.
+// Abstraction of DXIL Data Type Categories
+class DXILTypeCategory<string categoryStr> {
+ string Name = categoryStr;
+}
+
+// Concrete DXIL Data Type Categories that are supported
+foreach cat = ["Scalar", "Vector", "Matrix",
+ "Sampler", "Texture", "Buffer",
+ "Struct", "UserDefined", "Array", "StateObject"] in {
+ def cat : DXILTypeCategory<cat>;
+}
+
+// Pseudo DXIL Type category to represent data characteristics not supported
+// as DXIL types.
+def PseudoCategory : DXILTypeCategory<"PseudoCategory">;
+
+// Abstraction of DXIL Types
+class DXILType<DXILTypeCategory category, string name> {
+ DXILTypeCategory Category = category;
+ string Name = name;
+}
+
+// Supported DXIL Type categories
+class DXILScalarType<string typeNameString> : DXILType<Scalar, typeNameString>;
+class DXILVectorType<string typeNameString> : DXILType<Vector, typeNameString>;
+class DXILMatrixType<string typeNameString> : DXILType<Matrix, typeNameString>;
+class DXILSamplerType<string typeNameString> : DXILType<Sampler, typeNameString>;
+class DXILTextureType<string typeNameString> : DXILType<Texture, typeNameString>;
+class DXILBufferType<string typeNameString> : DXILType<Buffer, typeNameString>;
+class DXILStructType<string typeNameString> : DXILType<Struct, typeNameString>;
+class DXILUserDefinedType<string typeNameString> : DXILType<UserDefined, typeNameString>;
+class DXILArrayType<string typeNameString> : DXILType<Array, typeNameString>;
+class DXILStateObjectType<string typeNameString> : DXILType<StateObject, typeNameString>;
-def voidTy : LLVMType<isVoid>;
+// Concrete Pseudo Types
+
+// Overload - used to convey overload specification of DXIL operation parameters.
+def OverloadTy : DXILType<PseudoCategory, "overload">;
+
+// Handle - used to represent handle (i8*) type
+def HandleTy : DXILType<PseudoCategory, "handle">;
+
+// Resource - used to represent a resource
+def ResourceTy : DXILType<PseudoCategory, "resource">;
+
+// Concrete scalar types supported by DXIL operations.
+def VoidTy : DXILScalarType<"void">;
// Floating point types
-def f16Ty : LLVMType<f16>;
-def f32Ty : LLVMType<f32>;
-def f64Ty : LLVMType<f64>;
+def Float16Ty : DXILScalarType<"f16">;
+def Float32Ty : DXILScalarType<"f32">;
+def Float64Ty : DXILScalarType<"f64">;
// Integer types
-def i1Ty : LLVMType<i1>;
-def i8Ty : LLVMType<i8>;
-def i16Ty : LLVMType<i16>;
-def i32Ty : LLVMType<i32>;
-def i64Ty : LLVMType<i64>;
+def Int1Ty : DXILScalarType<"i1">;
+def Int8Ty : DXILScalarType<"i8">;
+def Int16Ty : DXILScalarType<"i16">;
+def Int32Ty : DXILScalarType<"i32">;
+def Int64Ty : DXILScalarType<"i64">;
+
+// Any of the above scalar types
+def AnyScalarTy : DXILScalarType<"anyScalar">;
+
+// Concrete buffer types
+def CBufferTy : DXILBufferType<"cbuffer">;
// The parameter description for a DXIL operation
-class DXILOpParameter<int pos, string type, string name, string doc,
+class DXILOpParameter<int pos, DXILType type, string name, string doc,
bit isConstant = 0, string enumName = "",
int maxValue = 0> {
int Pos = pos; // Position in parameter list
- string Type = type; // LLVM type name, $o for overload, $r for resource
- // type, $cb for legacy cbuffer, $u4 for u4 struct
+ DXILType ParamType = type; // Parameter type
string Name = name; // Short, unique parameter name
string Doc = doc; // Description of this parameter
bit IsConstant = isConstant; // Whether this parameter requires a constant value in the IR
@@ -74,7 +122,7 @@ class DXILOperationDesc {
DXILOpCategory OpCategory; // Category of the operation
string Doc = ""; // Description of the operation
list<DXILOpParameter> Params = []; // Parameter list of the operation
- list<LLVMType> OverloadTypes = []; // Overload types, if applicable
+ list<DXILType> OverloadTypes = []; // Overload types, if applicable
EnumAttr Attribute; // Operation Attribute. Leverage attributes defined in Attributes.td
// ReadNone - operation does not access memory.
// ReadOnly - only reads from memory.
@@ -91,7 +139,7 @@ class DXILOperationDesc {
}
class DXILOperation<string name, int opCode, DXILOpClass opClass, DXILOpCategory opCategory, string doc,
- list<LLVMType> oloadTypes, EnumAttr attrs, list<DXILOpParameter> params,
+ list<DXILScalarType> oloadTypes, EnumAttr attrs, list<DXILOpParameter> params,
list<string> statsGroup = []> : DXILOperationDesc {
let OpName = name;
let OpCode = opCode;
@@ -108,55 +156,55 @@ class DXILOperation<string name, int opCode, DXILOpClass opClass, DXILOpCategory
class LLVMIntrinsic<Intrinsic llvm_intrinsic_> { Intrinsic llvm_intrinsic = llvm_intrinsic_; }
def Sin : DXILOperation<"Sin", 13, UnaryClass, UnaryFloatCategory, "returns sine(theta) for theta in radians.",
- [f16Ty,f32Ty], ReadNone,
+ [Float16Ty,Float32Ty], ReadNone,
[
- DXILOpParameter<0, "$o", "", "operation result">,
- DXILOpParameter<1, "i32", "opcode", "DXIL opcode">,
- DXILOpParameter<2, "$o", "value", "input value">
+ DXILOpParameter<0, OverloadTy, "", "operation result">,
+ DXILOpParameter<1, Int32Ty, "opcode", "DXIL opcode">,
+ DXILOpParameter<2, OverloadTy, "value", "input value">
],
["floats"]>,
LLVMIntrinsic<int_sin>;
def UMax : DXILOperation< "UMax", 39, BinaryClass, BinaryUintCategory, "unsigned integer maximum. UMax(a,b) = a > b ? a : b",
- [i16Ty,i32Ty,i64Ty], ReadNone,
+ [Int16Ty,Int32Ty,Int64Ty], ReadNone,
[
- DXILOpParameter<0, "$o", "", "operation result">,
- DXILOpParameter<1, "i32", "opcode", "DXIL opcode">,
- DXILOpParameter<2, "$o", "a", "input value">,
- DXILOpParameter<3, "$o", "b", "input value">
+ DXILOpParameter<0, OverloadTy, "", "operation result">,
+ DXILOpParameter<1, Int32Ty, "opcode", "DXIL opcode">,
+ DXILOpParameter<2, OverloadTy, "a", "input value">,
+ DXILOpParameter<3, OverloadTy, "b", "input value">
],
["uints"]>,
LLVMIntrinsic<int_umax>;
-def ThreadId : DXILOperation< "ThreadId", 93, ThreadIdClass, ComputeIDCategory, "reads the thread ID", [i32Ty], ReadNone,
+def ThreadId : DXILOperation< "ThreadId", 93, ThreadIdClass, ComputeIDCategory, "reads the thread ID", [Int32Ty], ReadNone,
[
- DXILOpParameter<0, "i32", "", "thread ID component">,
- DXILOpParameter<1, "i32", "opcode", "DXIL opcode">,
- DXILOpParameter<2, "i32", "component", "component to read (x,y,z)">
+ DXILOpParameter<0, Int32Ty, "", "thread ID component">,
+ DXILOpParameter<1, Int32Ty, "opcode", "DXIL opcode">,
+ DXILOpParameter<2, Int32Ty, "component", "component to read (x,y,z)">
]>,
LLVMIntrinsic<int_dx_thread_id>;
-def GroupId : DXILOperation< "GroupId", 94, GroupIdClass, ComputeIDCategory, "reads the group ID (SV_GroupID)", [i32Ty], ReadNone,
+def GroupId : DXILOperation< "GroupId", 94, GroupIdClass, ComputeIDCategory, "reads the group ID (SV_GroupID)", [Int32Ty], ReadNone,
[
- DXILOpParameter<0, "i32", "", "group ID component">,
- DXILOpParameter<1, "i32", "opcode", "DXIL opcode">,
- DXILOpParameter<2, "i32", "component", "component to read">
+ DXILOpParameter<0, Int32Ty, "", "group ID component">,
+ DXILOpParameter<1, Int32Ty, "opcode", "DXIL opcode">,
+ DXILOpParameter<2, Int32Ty, "component", "component to read">
]>,
LLVMIntrinsic<int_dx_group_id>;
def ThreadIdInGroup : DXILOperation< "ThreadIdInGroup", 95, ThreadIdInGroupClass, ComputeIDCategory,
- "reads the thread ID within the group (SV_GroupThreadID)", [i32Ty], ReadNone,
+ "reads the thread ID within the group (SV_GroupThreadID)", [Int32Ty], ReadNone,
[
- DXILOpParameter<0, "i32", "", "thread ID in group component">,
- DXILOpParameter<1, "i32", "opcode", "DXIL opcode">,
- DXILOpParameter<2, "i32", "component", "component to read (x,y,z)">
+ DXILOpParameter<0, Int32Ty, "", "thread ID in group component">,
+ DXILOpParameter<1, Int32Ty, "opcode", "DXIL opcode">,
+ DXILOpParameter<2, Int32Ty, "component", "component to read (x,y,z)">
]>,
LLVMIntrinsic<int_dx_thread_id_in_group>;
def FlattenedThreadIdInGroup : DXILOperation< "FlattenedThreadIdInGroup", 96, FlattenedThreadIdInGroupClass, ComputeIDCategory,
- "provides a flattened index for a given thread within a given group (SV_GroupIndex)", [i32Ty], ReadNone,
+ "provides a flattened index for a given thread within a given group (SV_GroupIndex)", [Int32Ty], ReadNone,
[
- DXILOpParameter<0, "i32", "", "result">,
- DXILOpParameter<1, "i32", "opcode", "DXIL opcode">
+ DXILOpParameter<0, Int32Ty, "", "result">,
+ DXILOpParameter<1, Int32Ty, "opcode", "DXIL opcode">
]>,
LLVMIntrinsic<int_dx_flattened_thread_id_in_group>;
diff --git a/llvm/utils/TableGen/DXILEmitter.cpp b/llvm/utils/TableGen/DXILEmitter.cpp
index 3378a904ac404d..1c1407e96c89ac 100644
--- a/llvm/utils/TableGen/DXILEmitter.cpp
+++ b/llvm/utils/TableGen/DXILEmitter.cpp
@@ -74,44 +74,30 @@ struct DXILOperationDesc {
};
} // end anonymous namespace
-// Convert DXIL type name string to dxil::ParameterKind
-//
-// @param typeNameStr Type name string
-// @return ParameterKind as defined in llvm/Support/DXILABI.h
-static ParameterKind getDXILTypeNameToKind(StringRef typeNameStr) {
- return StringSwitch<ParameterKind>(typeNameStr)
- .Case("voidTy", ParameterKind::VOID)
- .Case("f16Ty", ParameterKind::HALF)
- .Case("f32Ty", ParameterKind::FLOAT)
- .Case("f64Ty", ParameterKind::DOUBLE)
- .Case("i1Ty", ParameterKind::I1)
- .Case("i8Ty", ParameterKind::I8)
- .Case("i16Ty", ParameterKind::I16)
- .Case("i32Ty", ParameterKind::I32)
- .Case("i64Ty", ParameterKind::I64)
- .Case("overloadTy", ParameterKind::OVERLOAD)
- .Case("handleTy", ParameterKind::DXIL_HANDLE)
- .Case("cbufferRetTy", ParameterKind::CBUFFER_RET)
- .Case("resourceRetTy", ParameterKind::RESOURCE_RET)
- .Default(ParameterKind::INVALID);
-}
+/*!
+ Convert DXIL type name string to dxil::ParameterKind
-static ParameterKind parameterTypeNameToKind(StringRef Name) {
- return StringSwitch<ParameterKind>(Name)
- .Case("void", ParameterKind::VOID)
- .Case("half", ParameterKind::HALF)
- .Case("float", ParameterKind::FLOAT)
- .Case("double", ParameterKind::DOUBLE)
- .Case("i1", ParameterKind::I1)
- .Case("i8", ParameterKind::I8)
- .Case("i16", ParameterKind::I16)
- .Case("i32", ParameterKind::I32)
- .Case("i64", ParameterKind::I64)
- .Case("$o", ParameterKind::OVERLOAD)
- .Case("dx.types.Handle", ParameterKind::DXIL_HANDLE)
- .Case("dx.types.CBufRet", ParameterKind::CBUFFER_RET)
- .Case("dx.types.ResRet", ParameterKind::RESOURCE_RET)
+ @param typeNameStr Type name string
+ @return ParameterKind As defined in llvm/Support/DXILABI.h
+*/
+static ParameterKind getDXILTypeNameToKind(StringRef typeNameStr) {
+ auto paramKind = StringSwitch<ParameterKind>(typeNameStr)
+ .Case("VoidTy", ParameterKind::VOID)
+ .Case("Float16Ty", ParameterKind::HALF)
+ .Case("Float32Ty", ParameterKind::FLOAT)
+ .Case("Float64Ty", ParameterKind::DOUBLE)
+ .Case("Int1Ty", ParameterKind::I1)
+ .Case("Int8Ty", ParameterKind::I8)
+ .Case("Int16Ty", ParameterKind::I16)
+ .Case("Int32Ty", ParameterKind::I32)
+ .Case("Int64Ty", ParameterKind::I64)
+ .Case("OverloadTy", ParameterKind::OVERLOAD)
+ .Case("HandleTy", ParameterKind::DXIL_HANDLE)
+ .Case("CbufferTy", ParameterKind::CBUFFER_RET)
+ .Case("ResourceTy", ParameterKind::RESOURCE_RET)
.Default(ParameterKind::INVALID);
+ assert( paramKind != ParameterKind::INVALID && "Unsupported DXIL Type specified");
+ return paramKind;
}
DXILOperationDesc::DXILOperationDesc(const Record *R) {
@@ -151,7 +137,7 @@ DXILOperationDesc::DXILOperationDesc(const Record *R) {
DXILParameter::DXILParameter(const Record *R) {
Name = R->getValueAsString("Name");
Pos = R->getValueAsInt("Pos");
- Kind = parameterTypeNameToKind(R->getValueAsString("Type"));
+ Kind = getDXILTypeNameToKind(R->getValue("ParamType")->getValue()->getAsString());
if (R->getValue("Doc"))
Doc = R->getValueAsString("Doc");
IsConst = R->getValueAsBit("IsConstant");
@@ -296,10 +282,12 @@ static void emitDXILIntrinsicMap(std::vector<DXILOperationDesc> &Ops,
OS << "\n";
}
-// Convert operation attribute string to Attribute enum
-//
-// @param Attr string reference
-// @return std::string Attribute enum string
+/*!
+ Convert operation attribute string to Attribute enum
+
+ @param Attr string reference
+ @return std::string Attribute enum string
+ */
static std::string emitDXILOperationAttr(StringRef Attr) {
return StringSwitch<std::string>(Attr)
.Case("ReadNone", "Attribute::ReadNone")
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
llvm-beanz
reviewed
Feb 14, 2024
There was a problem hiding this comment.
Parts of this change (like replacing strings with TableGen values) are awesome and I'm all on board.
I question whether we should have a DXIL type abstraction at all. DXIL types are effectively just a subset of valid LLVM types. We could use the LLVM type abstractions that we get from Intrinsics.td to provide consistency between DXIL.td and IntrinsicsDirectX.td.
llvm/lib/Target/DirectX/DXIL.td
Outdated
| def i16Ty : LLVMType<i16>; | ||
| def i32Ty : LLVMType<i32>; | ||
| def i64Ty : LLVMType<i64>; | ||
| def Int1Ty : DXILScalarType<"i1">; |
There was a problem hiding this comment.
I'm not sure I understand why we're making this change. DXIL types are a subset of LLVM IR types. My initial reaction to seeing this is that we're making our types different... but why?
I'm also unsure why we're not just using the type definitions from Intrinsics.td since we include it above.
I defined
|
bharadwajy
changed the title
|
Updated PR with a change that deletes |
bogner
approved these changes
Feb 14, 2024
python3kgae
reviewed
Feb 14, 2024
| DXILOpParameter<2, "$o", "value", "input value"> | ||
| DXILOpParameter<0, llvm_anyfloat_ty, "", "operation result">, | ||
| DXILOpParameter<1, llvm_i32_ty, "opcode", "DXIL opcode">, | ||
| DXILOpParameter<2, llvm_anyfloat_ty, "value", "input value"> |
There was a problem hiding this comment.
The $o for parameter 2 needs to be the same as parameter 0,
Is it possible to get something like LLVMMatchType?
There was a problem hiding this comment.
The $o for parameter 2 needs to be the same as parameter 0, Is it possible to get something like LLVMMatchType?
Good idea. Thanks!
However, I will need to investigate how such a constraint would be enforced in the backend code (??). Additionally, it appears that parameter types are expected to be specified as a list of LLVMTypes.
@python3kgae, would you be OK if the change you suggested is done in a subsequent PR?
python3kgae
approved these changes
Feb 21, 2024
llvm-beanz
approved these changes
Feb 21, 2024
| .Case("dxil_cbuffer_ty", ParameterKind::CBUFFER_RET) | ||
| .Case("dxil_resource_ty", ParameterKind::RESOURCE_RET) | ||
| .Default(ParameterKind::INVALID); | ||
| assert(paramKind != ParameterKind::INVALID && |
There was a problem hiding this comment.
This is a bit hacky and kinda violates the purpose of these being based on the LLVM type system.
I'm okay with this going in initially like this, but we should move this to being based on the underlying ValueType rather than string matching the name of the declaration.
There was a problem hiding this comment.
Agreed. Will change. Thanks.
|
@bogner, @llvm-beanz, @python3kgae can one of you please merge this PR? Thanks! |
7 tasks
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.
This change uniformly uses LLVM Types in the specification of parameter types and overload types of DXIL operation.
Updated (a) parameter types accordingly in the specification of existing DXILOperations and (b) DXILEmitter.