[DXIL] Add DXIL version-specific TableGen specification and implementation of DXIL Ops #97593
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
|
@llvm/pr-subscribers-backend-directx Author: S. Bharadwaj Yadavalli (bharadwajy) ChangesUpdate TableGen specification of DXIL Op records in DXIL.td per the currentdesign document.
Implement functionality to consume in TableGen backend, DXILEmitter, the above specification enhancements, and generate C++ code (in (DXILOperations.inc) that represents properties of DXIL Ops, associated type declarations and corresponding accessor functions. Add mtriple with the required shader model version to commandline of tests. Patch is 92.77 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/97593.diff 60 Files Affected:
diff --git a/llvm/lib/Target/DirectX/DXIL.td b/llvm/lib/Target/DirectX/DXIL.td
index adaaa2a6e0d4e..c78bd9ae81661 100644
--- a/llvm/lib/Target/DirectX/DXIL.td
+++ b/llvm/lib/Target/DirectX/DXIL.td
@@ -13,15 +13,45 @@
include "llvm/IR/Intrinsics.td"
-class DXILOpClass;
+// Abstract class to represent major and minor version values
+class Version<int major, int minor> {
+ int Major = major;
+ int Minor = minor;
+}
+
+// Valid Shader model version records
+
+// Shader Model 6.0 - 6.8 and DXIL Version 1.0 - 1.8
+foreach i = 0...8 in {
+ def SM6_#i : Version<6, i>;
+ def DX1_#i : Version<1, i>;
+}
+
+// Resource ValueType - has no size or value
+def resourceVT : ValueType<-1, -1>;
+
+// Resource type
+def dxil_resource_ty : LLVMType<resourceVT>;
+// Overload type alias of llvm_any_ty
+defvar dxil_overload_ty = llvm_any_ty;
-// Following is a set of DXIL Operation classes whose names appear to be
-// arbitrary, yet need to be a substring of the function name used during
-// lowering to DXIL Operation calls. These class name strings are specified
-// as the third argument of add_dixil_op in utils/hct/hctdb.py and case converted
-// in utils/hct/hctdb_instrhelp.py of DirectXShaderCompiler repo. The function
-// name has the format "dx.op.<class-name>.<return-type>".
+// DXIL Op attribute
+class DXILOpAttr;
+defset list<DXILOpAttr> OpAttributes = {
+ def ReadOnly : DXILOpAttr;
+ def ReadNone : DXILOpAttr;
+ def IsDerivative : DXILOpAttr;
+ def IsGradient : DXILOpAttr;
+ def IsFeedback : DXILOpAttr;
+ def IsWave : DXILOpAttr;
+ def NeedsUniformInputs : DXILOpAttr;
+ def IsBarrier : DXILOpAttr;
+}
+
+class DXILOpClass;
+
+// Concrete definitions of DXIL Op Classes
defset list<DXILOpClass> OpClasses = {
def acceptHitAndEndSearch : DXILOpClass;
def allocateNodeOutputRecords : DXILOpClass;
@@ -212,154 +242,576 @@ defset list<DXILOpClass> OpClasses = {
def UnknownOpClass: DXILOpClass;
}
-// Several of the overloaded DXIL Operations support for data types
-// that are a subset of the overloaded LLVM intrinsics that they map to.
-// For e.g., llvm.sin.* intrinsic operates on any floating-point type and
-// maps for lowering to DXIL Op Sin. However, valid overloads of DXIL Sin
-// operation overloads are half (f16) and float (f32) only.
-//
-// The following abstracts overload types specific to DXIL operations.
-
-class DXILType : LLVMType<OtherVT> {
- let isAny = 1;
- int isI16OrI32 = 0;
- int isHalfOrFloat = 0;
-}
-
-// Concrete records for various overload types supported specifically by
-// DXIL Operations.
-let isI16OrI32 = 1 in
- def llvm_i16ori32_ty : DXILType;
-
-let isHalfOrFloat = 1 in
- def llvm_halforfloat_ty : DXILType;
-
-// Abstraction DXIL Operation to LLVM intrinsic
-class DXILOpMappingBase {
- int OpCode = 0; // Opcode of DXIL Operation
- DXILOpClass OpClass = UnknownOpClass;// Class of DXIL Operation.
- Intrinsic LLVMIntrinsic = ?; // LLVM Intrinsic DXIL Operation maps to
- string Doc = ""; // A short description of the operation
- list<LLVMType> OpTypes = ?; // Valid types of DXIL Operation in the
- // format [returnTy, param1ty, ...]
-}
-
-class DXILOpMapping<int opCode, DXILOpClass opClass,
- Intrinsic intrinsic, string doc,
- list<LLVMType> opTys = []> : DXILOpMappingBase {
- int OpCode = opCode; // Opcode corresponding to DXIL Operation
- DXILOpClass OpClass = opClass; // Class of DXIL Operation.
- Intrinsic LLVMIntrinsic = intrinsic; // LLVM Intrinsic the DXIL Operation maps
- string Doc = doc; // to a short description of the operation
- list<LLVMType> OpTypes = !if(!eq(!size(opTys), 0), LLVMIntrinsic.Types, opTys);
-}
-
-// Concrete definition of DXIL Operation mapping to corresponding LLVM intrinsic
-def Abs : DXILOpMapping<6, unary, int_fabs,
- "Returns the absolute value of the input.">;
-def IsInf : DXILOpMapping<9, isSpecialFloat, int_dx_isinf,
- "Determines if the specified value is infinite.",
- [llvm_i1_ty, llvm_halforfloat_ty]>;
-def Cos : DXILOpMapping<12, unary, int_cos,
- "Returns cosine(theta) for theta in radians.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def Sin : DXILOpMapping<13, unary, int_sin,
- "Returns sine(theta) for theta in radians.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def Tan : DXILOpMapping<14, unary, int_tan,
- "Returns tangent(theta) for theta in radians.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def ACos : DXILOpMapping<15, unary, int_acos,
- "Returns the arccosine of each component of input.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def ASin : DXILOpMapping<16, unary, int_asin,
- "Returns the arcsine of each component of input.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def ATan : DXILOpMapping<17, unary, int_atan,
- "Returns the arctangent of each component of input.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def HCos : DXILOpMapping<18, unary, int_cosh,
- "Returns the hyperbolic cosine of the specified value.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def HSin : DXILOpMapping<19, unary, int_sinh,
- "Returns the hyperbolic sine of the specified value.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def HTan : DXILOpMapping<20, unary, int_tanh,
- "Returns the hyperbolic tan of the specified value.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-
-def Exp2 : DXILOpMapping<21, unary, int_exp2,
- "Returns the base 2 exponential, or 2**x, of the specified value."
- "exp2(x) = 2**x.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def Frac : DXILOpMapping<22, unary, int_dx_frac,
- "Returns a fraction from 0 to 1 that represents the "
- "decimal part of the input.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def Log2 : DXILOpMapping<23, unary, int_log2,
- "Returns the base-2 logarithm of the specified value.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def Sqrt : DXILOpMapping<24, unary, int_sqrt,
- "Returns the square root of the specified floating-point"
- "value, per component.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def RSqrt : DXILOpMapping<25, unary, int_dx_rsqrt,
- "Returns the reciprocal of the square root of the specified value."
- "rsqrt(x) = 1 / sqrt(x).",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def Round : DXILOpMapping<26, unary, int_roundeven,
- "Returns the input rounded to the nearest integer"
- "within a floating-point type.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def Floor : DXILOpMapping<27, unary, int_floor,
- "Returns the largest integer that is less than or equal to the input.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def Ceil : DXILOpMapping<28, unary, int_ceil,
- "Returns the smallest integer that is greater than or equal to the input.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def Trunc : DXILOpMapping<29, unary, int_trunc,
- "Returns the specified value truncated to the integer component.",
- [llvm_halforfloat_ty, LLVMMatchType<0>]>;
-def Rbits : DXILOpMapping<30, unary, int_bitreverse,
- "Returns the specified value with its bits reversed.",
- [llvm_anyint_ty, LLVMMatchType<0>]>;
-def FMax : DXILOpMapping<35, binary, int_maxnum,
- "Float maximum. FMax(a,b) = a > b ? a : b">;
-def FMin : DXILOpMapping<36, binary, int_minnum,
- "Float minimum. FMin(a,b) = a < b ? a : b">;
-def SMax : DXILOpMapping<37, binary, int_smax,
- "Signed integer maximum. SMax(a,b) = a > b ? a : b">;
-def SMin : DXILOpMapping<38, binary, int_smin,
- "Signed integer minimum. SMin(a,b) = a < b ? a : b">;
-def UMax : DXILOpMapping<39, binary, int_umax,
- "Unsigned integer maximum. UMax(a,b) = a > b ? a : b">;
-def UMin : DXILOpMapping<40, binary, int_umin,
- "Unsigned integer minimum. UMin(a,b) = a < b ? a : b">;
-def FMad : DXILOpMapping<46, tertiary, int_fmuladd,
- "Floating point arithmetic multiply/add operation. fmad(m,a,b) = m * a + b.">;
-def IMad : DXILOpMapping<48, tertiary, int_dx_imad,
- "Signed integer arithmetic multiply/add operation. imad(m,a,b) = m * a + b.">;
-def UMad : DXILOpMapping<49, tertiary, int_dx_umad,
- "Unsigned integer arithmetic multiply/add operation. umad(m,a,b) = m * a + b.">;
-let OpTypes = !listconcat([llvm_halforfloat_ty], !listsplat(llvm_halforfloat_ty, 4)) in
- def Dot2 : DXILOpMapping<54, dot2, int_dx_dot2,
- "dot product of two float vectors Dot(a,b) = a[0]*b[0] + ... + a[n]*b[n] where n is between 0 and 1">;
-let OpTypes = !listconcat([llvm_halforfloat_ty], !listsplat(llvm_halforfloat_ty, 6)) in
- def Dot3 : DXILOpMapping<55, dot3, int_dx_dot3,
- "dot product of two float vectors Dot(a,b) = a[0]*b[0] + ... + a[n]*b[n] where n is between 0 and 2">;
-let OpTypes = !listconcat([llvm_halforfloat_ty], !listsplat(llvm_halforfloat_ty, 8)) in
- def Dot4 : DXILOpMapping<56, dot4, int_dx_dot4,
- "dot product of two float vectors Dot(a,b) = a[0]*b[0] + ... + a[n]*b[n] where n is between 0 and 3">;
-def ThreadId : DXILOpMapping<93, threadId, int_dx_thread_id,
- "Reads the thread ID">;
-def GroupId : DXILOpMapping<94, groupId, int_dx_group_id,
- "Reads the group ID (SV_GroupID)">;
-def ThreadIdInGroup : DXILOpMapping<95, threadIdInGroup,
- int_dx_thread_id_in_group,
- "Reads the thread ID within the group "
- "(SV_GroupThreadID)">;
-def FlattenedThreadIdInGroup : DXILOpMapping<96, flattenedThreadIdInGroup,
- int_dx_flattened_thread_id_in_group,
- "Provides a flattened index for a "
- "given thread within a given "
- "group (SV_GroupIndex)">;
+// Shader stages
+class ShaderStage;
+
+defset list<ShaderStage> ShaderStages = {
+ def compute : ShaderStage;
+ def domain : ShaderStage;
+ def hull : ShaderStage;
+ def pixel : ShaderStage;
+ def vertex : ShaderStage;
+ def geometry : ShaderStage;
+ def library : ShaderStage;
+ def amplification : ShaderStage;
+ def mesh : ShaderStage;
+ def node : ShaderStage;
+ def raygeneration : ShaderStage;
+ def intersection : ShaderStage;
+ def allKinds : ShaderStage;
+}
+
+// Primitive predicate
+class Pred;
+
+// Shader Model version predicate. This translates to
+// a check for specified shader model version
+class SMVersion<Version ver> : Pred {
+ Version sm_version = ver;
+}
+
+// Class abstraction of constraints predicated on Shader Model version
+class SMVersionConstraints<Version ver, dag oloads, dag stages> : SMVersion<ver> {
+ dag overload_types = oloads;
+ dag stage_kinds = stages;
+}
+
+// Marker used to identify argument list.
+def ins;
+
+// Marker used to identify result list.
+def out;
+
+// Marker used to identify list of shader model based attributes.
+def sm_attrs;
+
+// Marker used to identify overload types list.
+def overloads;
+
+// Marker used to identify stage kinds list.
+def stages;
+
+// Marker used to identify attribute list.
+def attrs;
+
+// Abstraction DXIL Operation
+class DXILOp {
+ // A short description of the operation
+ string Doc = "";
+
+ // Opcode of DXIL Operation
+ int OpCode = 0;
+
+ // Class of DXIL Operation.
+ DXILOpClass OpClass = UnknownOpClass;
+
+ // LLVM Intrinsic DXIL Operation maps to
+ Intrinsic LLVMIntrinsic = ?;
+
+ // TODO : DELETE THIS once support in DXILEmitter is added to consume
+ // overload_types and generate appropriate code.
+ // Valid overload type of DXIL Operation
+ list<LLVMType> OpOverloadTypes = ?;
+
+ // Dag containing the arguments of the op. Default to 0 arguments.
+ dag arguments = (ins);
+
+ // Results of the op. Default to 0 results.
+ dag result = (out);
+
+ // List of constraints predicated on Shader Model version
+ // This field is required to be specified. If a DXIL Op has no
+ // overloads or stages predicated on Shader Model version, the
+ // minimum Shader Model version the DXIL Op is supported it
+ // should be specified as a single list item
+ // [SMVersionConstraints<SMX_Y, (overloads), (stages allKinds)]
+ // If the DXIL Op is a DXIL Op that is not predicted on Shader
+ // Model version, it should be specified as an empty list.
+
+ list<SMVersionConstraints> sm_constraints;
+
+ // Non-predicated operation attributes
+ dag attributes = (attrs);
+ Version DXILVersion = ?;
+}
+
+// Concrete definitions of DXIL Operations
+
+def IsInf : DXILOp {
+ let Doc = "Determines if the specified value is infinite.";
+ let OpCode = 9;
+ let OpClass = isSpecialFloat;
+ let LLVMIntrinsic = int_dx_isinf;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty];
+ let arguments = (ins llvm_anyfloat_ty);
+ let result = (out llvm_i1_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty),
+ (stages allKinds)>];
+}
+
+def Abs : DXILOp {
+ let Doc = "Returns the absolute value of the input.";
+ let OpCode = 6;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_fabs;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty, llvm_double_ty];
+ let arguments = (ins LLVMMatchType<0>);
+ let result = (out dxil_overload_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty, llvm_double_ty),
+ (stages allKinds)>];
+}
+
+def Cos : DXILOp {
+ let Doc ="Returns cosine(theta) for theta in radians.";
+ let OpCode = 12;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_cos;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty];
+ let arguments = (ins LLVMMatchType<0>);
+ let result = (out dxil_overload_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty),
+ (stages allKinds)>];
+}
+
+def Sin : DXILOp {
+ let Doc ="Returns sine(theta) for theta in radians.";
+ let OpCode = 13;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_sin;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty];
+ let arguments = (ins LLVMMatchType<0>);
+ let result = (out dxil_overload_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty),
+ (stages allKinds)>];
+ let attributes = (attrs ReadNone);
+ let DXILVersion = DX1_0;
+}
+
+def Tan : DXILOp {
+ let Doc = "Returns tangent(theta) for theta in radians.";
+ let OpCode = 14;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_tan;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty];
+ let arguments = (ins LLVMMatchType<0>);
+ let result = (out dxil_overload_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty),
+ (stages allKinds)>];
+}
+
+def ACos : DXILOp {
+ let Doc = "Returns the arccosine of the specified value.";
+ let OpCode = 15;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_acos;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty];
+ let arguments = (ins LLVMMatchType<0>);
+ let result = (out dxil_overload_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty),
+ (stages allKinds)>];
+}
+
+def ASin : DXILOp {
+ let Doc = "Returns the arcsine of the specified value.";
+ let OpCode = 16;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_asin;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty];
+ let arguments = (ins LLVMMatchType<0>);
+ let result = (out dxil_overload_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty),
+ (stages allKinds)>];
+}
+
+def ATan : DXILOp {
+ let Doc = "Returns the arctangent of the specified value.";
+ let OpCode = 17;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_atan;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty];
+ let arguments = (ins LLVMMatchType<0>);
+ let result = (out dxil_overload_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty),
+ (stages allKinds)>];
+}
+
+def HCos : DXILOp {
+ let Doc = "Returns the hyperbolic cosine of the specified value.";
+ let OpCode = 18;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_cosh;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty];
+ let arguments = (ins LLVMMatchType<0>);
+ let result = (out dxil_overload_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty),
+ (stages allKinds)>];
+}
+
+def HSin : DXILOp {
+ let Doc = "Returns the hyperbolic sine of the specified value.";
+ let OpCode = 19;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_sinh;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty];
+ let arguments = (ins LLVMMatchType<0>);
+ let result = (out dxil_overload_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty),
+ (stages allKinds)>];
+}
+
+def HTan : DXILOp {
+ let Doc = "Returns the hyperbolic tan of the specified value.";
+ let OpCode = 20;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_tanh;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty];
+ let arguments = (ins LLVMMatchType<0>);
+ let result = (out dxil_overload_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty),
+ (stages allKinds)>];
+}
+
+def Exp2 : DXILOp {
+ let Doc = "Returns the base 2 exponential, or 2**x, of the specified value. exp2(x) = 2**x.";
+ let OpCode = 21;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_exp2;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty];
+ let arguments = (ins LLVMMatchType<0>);
+ let result = (out dxil_overload_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty),
+ (stages allKinds)>];
+}
+
+def Frac : DXILOp {
+ let Doc = "Returns a fraction from 0 to 1 that represents the decimal part of the input.";
+ let OpCode = 22;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_dx_frac;
+ let OpOverloadTypes = [llvm_half_ty, llvm_float_ty];
+ let arguments = (ins LLVMMatchType<0>);
+ let result = (out dxil_overload_ty);
+ let sm_constraints = [SMVersionConstraints<SM6_0,
+ (overloads llvm_half_ty, llvm_float_ty),
+ (stages allKinds)>];
+}
+
+def Log2 : DXILOp {
+ let Doc = "Returns the base-2 logarithm of the specified value.";
+ let OpCode = 23;
+ let OpClass = unary;
+ let LLVMIntrinsic = int_log2;
+...
[truncated]
|
python3kgae
reviewed
Jul 3, 2024
python3kgae
reviewed
Jul 3, 2024
python3kgae
reviewed
Jul 3, 2024
python3kgae
reviewed
Jul 3, 2024
bogner
reviewed
Jul 3, 2024
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
bharadwajy
force-pushed
the
dxil_td/constraint-specification
branch
2 times, most recently
from
b581f9f
to
4b69099
Compare
bharadwajy
commented
Jul 9, 2024
python3kgae
reviewed
Jul 9, 2024
python3kgae
reviewed
Jul 11, 2024
python3kgae
reviewed
Jul 11, 2024
damyanp
reviewed
Jul 12, 2024
damyanp
reviewed
Jul 12, 2024
llvm-beanz
reviewed
Jul 12, 2024
damyanp
reviewed
Jul 12, 2024
| default: | ||
| break; | ||
| } | ||
| llvm_unreachable( |
There was a problem hiding this comment.
Should this be a diagnostic? I notice in the place that this function is called, UnknownEnvironment will cause a diagnostic.
llvm-beanz
reviewed
Jul 12, 2024
damyanp
reviewed
Jul 12, 2024
llvm-beanz
reviewed
Jul 12, 2024
damyanp
reviewed
Jul 12, 2024
|
|
||
| // Ensure valid shader stage constraints are specified | ||
| if (ValidShaderKindMask == ShaderKind::Unknown) { | ||
| report_fatal_error( |
There was a problem hiding this comment.
The clang code seems to have a diagnostic td file that these messages go into. Is it normal practice not to do that in llvm? Or is this more about the types of errors that mean they don't go into a centralized table?
There was a problem hiding this comment.
There isn't really an equivalent to Clang's diagnostic tablegen in LLVM. Generally LLVM's errors are all fatal so they report this way (basically exiting the compiler).
We may want to consider a larger design proposal for LLVM to allow passes to propagate errors up. This has been discussed in the past, but never really worked on.
There was a problem hiding this comment.
There are so-called "Backend Diagnostics" in llvm/IR/DiagnosticInfo.h, which is probably what we should actually be using here since it can show source locations, though it does mostly have the same hand-rolled-strings issue as this. However:
- We've discussed making sure we have good diagnostics from the frontend for misusing APIs, so we should only get here if we're starting from LLVM IR
- That kind of diagnostic should really be coming out of the DXILOpLowering pass and not from inside the DXILOpBuidler anyway
For now I think the assert/fatal error approach is probably fine while we get things propped up a little better, but we will want to improve this over time.
damyanp
reviewed
Jul 12, 2024
damyanp
reviewed
Jul 12, 2024
damyanp
reviewed
Jul 12, 2024
damyanp
requested changes
Jul 15, 2024
bharadwajy
force-pushed
the
dxil_td/constraint-specification
branch
from
34fe63b
to
05669d4
Compare
bharadwajy
changed the title
bharadwajy
changed the title
bharadwajy
force-pushed
the
dxil_td/constraint-specification
branch
from
61ffa47
to
a1155f4
Compare
llvm-beanz
reviewed
Jul 26, 2024
llvm-beanz
reviewed
Jul 26, 2024
llvm-beanz
reviewed
Jul 26, 2024
bharadwajy
force-pushed
the
dxil_td/constraint-specification
branch
from
ffc2f15
to
962d8a8
Compare
bharadwajy
commented
Jul 29, 2024
bogner
approved these changes
Jul 30, 2024
There was a problem hiding this comment.
This looks mostly okay. The error handling is awkward and the way that OpCodeProperty is defined and used is not very idiomatic tablegen and the whole thing is fairly unwieldy, which are both things we'll need to fix. This change is just adding to the existing pattern though, so it's probably best to deal with that in follow ups.
llvm/lib/Target/DirectX/DXIL.td
Outdated
| defvar halfTy = llvm_half_ty; | ||
| defvar floatTy = llvm_float_ty; | ||
| defvar doubleTy = llvm_double_ty; | ||
| defvar anyfloatTy = llvm_anyfloat_ty; |
There was a problem hiding this comment.
I don't think we need anyfloatTy - everywhere that it's used currently is really just overloadTy
There was a problem hiding this comment.
I don't think we need
anyfloatTy- everywhere that it's used currently is really justoverloadTy
Replaced anyfloatTy with overloadTy.
llvm/utils/TableGen/DXILEmitter.cpp
Outdated
| if (Recs.empty()) { | ||
| report_fatal_error(Twine("Atleast one specification of valid stage for ") + | ||
| OpName + " is required", | ||
| /* gen_crash_diag=*/false); |
There was a problem hiding this comment.
"llvm/TableGen/Error.h" has PrintFatalError, which is better because it can print a source location. We should use that for all of the errors in DXILEmitter.
There was a problem hiding this comment.
"llvm/TableGen/Error.h" has
PrintFatalError, which is better because it can print a source location. We should use that for all of the errors in DXILEmitter.
Changed usage of report_fatal_error() to PrintFatalError()
| @@ -42,6 +48,7 @@ class DXILOpBuilder { | |||
| private: | |||
| Module &M; | |||
| IRBuilderBase &B; | |||
| Triple TargetTriple; | |||
There was a problem hiding this comment.
It will simplify things to have the DXIL version and shader stage as members instead of the triple, and then we can just assume that they're valid like so:
DXILOpBuilder::DXILOpBuilder(Module &M, IRBuilderBase &B) : M(M), B(B) {
Triple TT(M.getTargetTriple());
DXILVersion = TT.getDXILVersion();
assert(TT.isShaderStageEnvironment() &&
"Cannot build DXIL ops outside of shader environment");
ShaderStage = TT.getEnvironment();
}It shouldn't be DXILOpBuilder's job to validate the triple.
There was a problem hiding this comment.
It will simplify things to have the DXIL version and shader stage as members instead of the triple, and then we can just assume that they're valid like so:
DXILOpBuilder::DXILOpBuilder(Module &M, IRBuilderBase &B) : M(M), B(B) { Triple TT(M.getTargetTriple()); DXILVersion = TT.getDXILVersion(); assert(TT.isShaderStageEnvironment() && "Cannot build DXIL ops outside of shader environment"); ShaderStage = TT.getEnvironment(); }It shouldn't be DXILOpBuilder's job to validate the triple.
Changed.
|
|
||
| // Ensure valid shader stage constraints are specified | ||
| if (ValidShaderKindMask == ShaderKind::Unknown) { | ||
| report_fatal_error( |
There was a problem hiding this comment.
There are so-called "Backend Diagnostics" in llvm/IR/DiagnosticInfo.h, which is probably what we should actually be using here since it can show source locations, though it does mostly have the same hand-rolled-strings issue as this. However:
- We've discussed making sure we have good diagnostics from the frontend for misusing APIs, so we should only get here if we're starting from LLVM IR
- That kind of diagnostic should really be coming out of the DXILOpLowering pass and not from inside the DXILOpBuidler anyway
For now I think the assert/fatal error approach is probably fine while we get things propped up a little better, but we will want to improve this over time.
- Delete anyFloatTy; Replace report_fatal_error with PrintFatalError - Stash DXILVersion and ShaderStage information from Module in DXILOpBuilder object.
python3kgae
approved these changes
Jul 30, 2024
MaskRay
added a commit
that referenced
this pull request
Jul 30, 2024
clementval
pushed a commit
to clementval/llvm-project
that referenced
this pull request
Jul 31, 2024
…ation of DXIL Ops (llvm#97593) Update TableGen specification of DXIL Op records in DXIL.td per the current design document. - Facilitate specification of overloads, shader stage and attributes predicated on DXIL Ops predicated DXIL version. Implement functionality to consume in TableGen backend, DXILEmitter, the above specification enhancements, and generate C++ code (in (DXILOperations.inc) that represents properties of DXIL Ops, associated type declarations and corresponding accessor functions. Changes to DXIL Op Lowering pass to consume the DXIL Op representation generated by the TableGen back end. Add mtriple with the required shader model version to commandline of tests.
clementval
pushed a commit
to clementval/llvm-project
that referenced
this pull request
Jul 31, 2024
bogner
added a commit
to bogner/llvm-project
that referenced
this pull request
Aug 3, 2024
As of cdfd884 "[DXIL] Add DXIL version-specific TableGen specification and implementation of DXIL Ops (llvm#97593)", all of these tests need to specify triples.
bogner
added a commit
that referenced
this pull request
Aug 3, 2024
banach-space
pushed a commit
to banach-space/llvm-project
that referenced
this pull request
Aug 7, 2024
…ation of DXIL Ops (llvm#97593) Update TableGen specification of DXIL Op records in DXIL.td per the current design document. - Facilitate specification of overloads, shader stage and attributes predicated on DXIL Ops predicated DXIL version. Implement functionality to consume in TableGen backend, DXILEmitter, the above specification enhancements, and generate C++ code (in (DXILOperations.inc) that represents properties of DXIL Ops, associated type declarations and corresponding accessor functions. Changes to DXIL Op Lowering pass to consume the DXIL Op representation generated by the TableGen back end. Add mtriple with the required shader model version to commandline of tests.
banach-space
pushed a commit
to banach-space/llvm-project
that referenced
this pull request
Aug 7, 2024
banach-space
pushed a commit
to banach-space/llvm-project
that referenced
this pull request
Aug 7, 2024
As of cdfd884 "[DXIL] Add DXIL version-specific TableGen specification and implementation of DXIL Ops (llvm#97593)", all of these tests need to specify triples.
kstoimenov
pushed a commit
to kstoimenov/llvm-project
that referenced
this pull request
Aug 15, 2024
As of cdfd884 "[DXIL] Add DXIL version-specific TableGen specification and implementation of DXIL Ops (llvm#97593)", all of these tests need to specify triples.
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.
Update TableGen specification of DXIL Op records in DXIL.td per the current design document.
Implement functionality to consume in TableGen backend, DXILEmitter, the above specification enhancements, and generate C++ code (in (DXILOperations.inc) that represents properties of DXIL Ops, associated type declarations and corresponding accessor functions.
Changes to DXIL Op Lowering pass to consume the DXIL Op representation generated by the TableGen back end.
Add mtriple with the required shader model version to commandline of tests.