Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DirectX][NFC] Change all DXIL TableGen tokens to CamelCase #80714

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

bharadwajy
Copy link
Contributor

These changes are in preparation for potential improvement of DXIL operation description and addition of more DXIL operations to DXIL.td.

@bharadwajy
Copy link
Contributor Author

llvm/lib/Target/DirectX/DXIL.td Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Feb 5, 2024

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 5, 2024

@llvm/pr-subscribers-backend-directx

Author: S. Bharadwaj Yadavalli (bharadwajy)

Changes

These changes are in preparation for potential improvement of DXIL operation description and addition of more DXIL operations to DXIL.td.


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

2 Files Affected:

  • (modified) llvm/lib/Target/DirectX/DXIL.td (+98-97)
  • (modified) llvm/utils/TableGen/DXILEmitter.cpp (+18-18)
diff --git a/llvm/lib/Target/DirectX/DXIL.td b/llvm/lib/Target/DirectX/DXIL.td
index 709279889653b8..340085f047d44c 100644
--- a/llvm/lib/Target/DirectX/DXIL.td
+++ b/llvm/lib/Target/DirectX/DXIL.td
@@ -7,138 +7,139 @@
 //===----------------------------------------------------------------------===//
 ///
 /// \file
-/// This is a target description file for DXIL operation.
+/// This is a target description file for DXIL operations.
 ///
 //===----------------------------------------------------------------------===//
 
 include "llvm/IR/Intrinsics.td"
 
-class dxil_class<string _name> {
-  string name = _name;
+// Abstract representation of the class a DXIL Operation belongs to.
+class DXILOpClass<string name> {
+  string Name = name;
 }
-class dxil_category<string _name> {
-  string name = _name;
+
+// Abstract representation of the category a DXIL Operation belongs to
+class DXILOpCategory<string name> {
+  string Name = name;
 }
 
-def Unary : dxil_class<"Unary">;
-def Binary : dxil_class<"Binary">;
-def FlattenedThreadIdInGroupClass : dxil_class<"FlattenedThreadIdInGroup">;
-def ThreadIdInGroupClass : dxil_class<"ThreadIdInGroup">;
-def ThreadIdClass : dxil_class<"ThreadId">;
-def GroupIdClass : dxil_class<"GroupId">;
-
-def binary_uint : dxil_category<"Binary uint">;
-def unary_float : dxil_category<"Unary float">;
-def ComputeID : dxil_category<"Compute/Mesh/Amplification shader">;
-
-
-// The parameter description for a DXIL instruction
-class dxil_param<int _pos, string type, string _name, string _doc,
-                 bit _is_const = 0, string _enum_name = "",
-                 int _max_value = 0> {
-  int pos = _pos;           // position in parameter list
-  string llvm_type = type; // llvm type name, $o for overload, $r for resource
-                           // type, $cb for legacy cbuffer, $u4 for u4 struct
-  string name = _name;      // short, unique name
-  string doc = _doc;        // the documentation description of this parameter
-  bit is_const =
-      _is_const; // whether this argument requires a constant value in the IR
-  string enum_name = _enum_name; // the name of the enum type if applicable
-  int max_value =
-      _max_value; // the maximum value for this parameter if applicable
+def UnaryClass : DXILOpClass<"Unary">;
+def BinaryClass : DXILOpClass<"Binary">;
+def FlattenedThreadIdInGroupClass : DXILOpClass<"FlattenedThreadIdInGroup">;
+def ThreadIdInGroupClass : DXILOpClass<"ThreadIdInGroup">;
+def ThreadIdClass : DXILOpClass<"ThreadId">;
+def GroupIdClass : DXILOpClass<"GroupId">;
+
+def BinaryUintCategory : DXILOpCategory<"Binary uint">;
+def UnaryFloatCategory : DXILOpCategory<"Unary float">;
+def ComputeIDCategory : DXILOpCategory<"Compute/Mesh/Amplification shader">;
+
+// The parameter description for a DXIL operation
+class DXILOpParameter<int pos, string type, string name, string doc,
+                 bit isConstant = 0, string enumName = "",
+                 int maxValue = 0> {
+  int Pos = pos;               // Position in parameter list
+  string LLVMType = type;      // LLVM type name, $o for overload, $r for resource
+                               // type, $cb for legacy cbuffer, $u4 for u4 struct
+  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
+  string EnumName = enumName;  // Name of the enum type, if applicable
+  int MaxValue = maxValue;     // Maximum value for this parameter, if applicable
 }
 
-// A representation for a DXIL instruction
-class dxil_inst<string _name> {
-  string name = _name; // short, unique name
-
-  string dxil_op = "";       // name of DXIL operation
-  int dxil_opid = 0;         // ID of DXIL operation
-  dxil_class  op_class;      // name of the opcode class
-  dxil_category category;    // classification for this instruction
-  string doc = "";           // the documentation description of this instruction
-  list<dxil_param> ops = []; // the operands that this instruction takes
-  string oload_types = "";   // overload types if applicable
-  string fn_attr = "";       // attribute shorthands: rn=does not access
-                             // memory,ro=only reads from memory,
-  bit is_deriv = 0;          // whether this is some kind of derivative
-  bit is_gradient = 0;       // whether this requires a gradient calculation
-  bit is_feedback = 0;       // whether this is a sampler feedback op
-  bit is_wave = 0; // whether this requires in-wave, cross-lane functionality
-  bit requires_uniform_inputs = 0; // whether this operation requires that all
-                                   // of its inputs are uniform across the wave
-  // Group dxil operation for stats.
-  // Like how many atomic/float/uint/int/... instructions used in the program.
-  list<string> stats_group = [];
+// A representation for a DXIL operation
+class DXILOperationDesc<string name> {
+  // TODO : Appears redundant. OpName should serve the same purpose
+  string Name = name; // short, unique name
+
+  string OpName = "";         // Name of DXIL operation
+  int OpCode = 0;             // Unique non-negative integer associated with the operation
+  DXILOpClass  OpClass;       // Class of the operation
+  DXILOpCategory OpCategory;  // Category of the operation
+  string Doc = "";            // Description of the operation
+  list<DXILOpParameter> Params = []; // Parameter list of the operation
+  string OverloadTypes = "";  // Overload types, if applicable
+  string Attributes = "";     // Attribute shorthands: rn=does not access
+                              // memory,ro=only reads from memory,
+  bit IsDerivative = 0;       // Whether this is some kind of derivative
+  bit IsGradient = 0;         // Whether this requires a gradient calculation
+  bit IsFeedback = 0;         // Whether this is a sampler feedback operation
+  bit IsWave = 0;             // Whether this requires in-wave, cross-lane functionality
+  bit NeedsUniformInputs = 0; // Whether this operation requires that all
+                              // of its inputs are uniform across the wave
+  // Group DXIL operation for stats - e.g., to accumulate the number of atomic/float/uint/int/...
+  // operations used in the program.
+  list<string> StatsGroup = [];
 }
 
-class dxil_op<string name, int code_id, dxil_class code_class, dxil_category op_category, string _doc,
-              string _oload_types, string _fn_attr, list<dxil_param> op_params,
-              list<string> _stats_group = []> : dxil_inst<name> {
-  let dxil_op = name;
-  let dxil_opid = code_id;
-  let doc = _doc;
-  let ops = op_params;
-  let op_class = code_class;
-  let category = op_category;
-  let oload_types = _oload_types;
-  let fn_attr = _fn_attr;
-  let stats_group = _stats_group;
+class DXILOperation<string name, int opCode, DXILOpClass opClass, DXILOpCategory opCategory, string doc,
+              string oloadTypes, string attrs, list<DXILOpParameter> params,
+              list<string> statsGroup = []> : DXILOperationDesc<name> {
+  let OpName = name;
+  let OpCode = opCode;
+  let Doc = doc;
+  let Params = params;
+  let OpClass = opClass;
+  let OpCategory = opCategory;
+  let OverloadTypes = oloadTypes;
+  let Attributes = attrs;
+  let StatsGroup = statsGroup;
 }
 
-// The intrinsic which map directly to this dxil op.
-class dxil_map_intrinsic<Intrinsic llvm_intrinsic_> { Intrinsic llvm_intrinsic = llvm_intrinsic_; }
+// LLVM intrinsic that DXIL operation maps to.
+class LLVMIntrinsic<Intrinsic llvm_intrinsic_> { Intrinsic llvm_intrinsic = llvm_intrinsic_; }
 
-def Sin : dxil_op<"Sin", 13, Unary, unary_float, "returns sine(theta) for theta in radians.",
+def Sin : DXILOperation<"Sin", 13, UnaryClass, UnaryFloatCategory, "returns sine(theta) for theta in radians.",
   "half;float;", "rn",
   [
-    dxil_param<0, "$o", "", "operation result">,
-    dxil_param<1, "i32", "opcode", "DXIL opcode">,
-    dxil_param<2, "$o", "value", "input value">
+    DXILOpParameter<0, "$o", "", "operation result">,
+    DXILOpParameter<1, "i32", "opcode", "DXIL opcode">,
+    DXILOpParameter<2, "$o", "value", "input value">
   ],
   ["floats"]>,
-  dxil_map_intrinsic<int_sin>;
+  LLVMIntrinsic<int_sin>;
 
-def UMax :dxil_op< "UMax", 39,  Binary,  binary_uint, "unsigned integer maximum. UMax(a,b) = a > b ? a : b",
+def UMax : DXILOperation< "UMax", 39,  BinaryClass,  BinaryUintCategory, "unsigned integer maximum. UMax(a,b) = a > b ? a : b",
     "i16;i32;i64;",  "rn",
   [
-    dxil_param<0,  "$o",  "",  "operation result">,
-    dxil_param<1,  "i32",  "opcode",  "DXIL opcode">,
-    dxil_param<2,  "$o",  "a",  "input value">,
-    dxil_param<3,  "$o",  "b",  "input value">
+    DXILOpParameter<0,  "$o",  "",  "operation result">,
+    DXILOpParameter<1,  "i32",  "opcode",  "DXIL opcode">,
+    DXILOpParameter<2,  "$o",  "a",  "input value">,
+    DXILOpParameter<3,  "$o",  "b",  "input value">
   ],
   ["uints"]>,
-  dxil_map_intrinsic<int_umax>;
+  LLVMIntrinsic<int_umax>;
 
-def ThreadId :dxil_op< "ThreadId", 93,  ThreadIdClass, ComputeID, "reads the thread ID", "i32;",  "rn",
+def ThreadId : DXILOperation< "ThreadId", 93,  ThreadIdClass, ComputeIDCategory, "reads the thread ID", "i32;",  "rn",
   [
-    dxil_param<0,  "i32",  "",  "thread ID component">,
-    dxil_param<1,  "i32",  "opcode",  "DXIL opcode">,
-    dxil_param<2,  "i32",  "component",  "component to read (x,y,z)">
+    DXILOpParameter<0,  "i32",  "",  "thread ID component">,
+    DXILOpParameter<1,  "i32",  "opcode",  "DXIL opcode">,
+    DXILOpParameter<2,  "i32",  "component",  "component to read (x,y,z)">
   ]>,
-  dxil_map_intrinsic<int_dx_thread_id>;
+  LLVMIntrinsic<int_dx_thread_id>;
 
-def GroupId :dxil_op< "GroupId", 94,  GroupIdClass, ComputeID, "reads the group ID (SV_GroupID)", "i32;",  "rn",
+def GroupId : DXILOperation< "GroupId", 94,  GroupIdClass, ComputeIDCategory, "reads the group ID (SV_GroupID)", "i32;",  "rn",
   [
-    dxil_param<0,  "i32",  "",  "group ID component">,
-    dxil_param<1,  "i32",  "opcode",  "DXIL opcode">,
-    dxil_param<2,  "i32",  "component",  "component to read">
+    DXILOpParameter<0,  "i32",  "",  "group ID component">,
+    DXILOpParameter<1,  "i32",  "opcode",  "DXIL opcode">,
+    DXILOpParameter<2,  "i32",  "component",  "component to read">
   ]>,
-  dxil_map_intrinsic<int_dx_group_id>;
+  LLVMIntrinsic<int_dx_group_id>;
 
-def ThreadIdInGroup :dxil_op< "ThreadIdInGroup", 95,  ThreadIdInGroupClass, ComputeID,
+def ThreadIdInGroup : DXILOperation< "ThreadIdInGroup", 95,  ThreadIdInGroupClass, ComputeIDCategory,
   "reads the thread ID within the group (SV_GroupThreadID)", "i32;",  "rn",
   [
-    dxil_param<0,  "i32",  "",  "thread ID in group component">,
-    dxil_param<1,  "i32",  "opcode",  "DXIL opcode">,
-    dxil_param<2,  "i32",  "component",  "component to read (x,y,z)">
+    DXILOpParameter<0,  "i32",  "",  "thread ID in group component">,
+    DXILOpParameter<1,  "i32",  "opcode",  "DXIL opcode">,
+    DXILOpParameter<2,  "i32",  "component",  "component to read (x,y,z)">
   ]>,
-  dxil_map_intrinsic<int_dx_thread_id_in_group>;
+  LLVMIntrinsic<int_dx_thread_id_in_group>;
 
-def FlattenedThreadIdInGroup :dxil_op< "FlattenedThreadIdInGroup", 96,  FlattenedThreadIdInGroupClass, ComputeID,
+def FlattenedThreadIdInGroup : DXILOperation< "FlattenedThreadIdInGroup", 96,  FlattenedThreadIdInGroupClass, ComputeIDCategory,
    "provides a flattened index for a given thread within a given group (SV_GroupIndex)", "i32;",  "rn",
   [
-    dxil_param<0,  "i32",  "",  "result">,
-    dxil_param<1,  "i32",  "opcode",  "DXIL opcode">
+    DXILOpParameter<0,  "i32",  "",  "result">,
+    DXILOpParameter<1,  "i32",  "opcode",  "DXIL opcode">
   ]>,
-  dxil_map_intrinsic<int_dx_flattened_thread_id_in_group>;
+  LLVMIntrinsic<int_dx_flattened_thread_id_in_group>;
diff --git a/llvm/utils/TableGen/DXILEmitter.cpp b/llvm/utils/TableGen/DXILEmitter.cpp
index ddc7cfb8134470..918c9fbfce0d19 100644
--- a/llvm/utils/TableGen/DXILEmitter.cpp
+++ b/llvm/utils/TableGen/DXILEmitter.cpp
@@ -72,11 +72,11 @@ struct DXILOperationData {
                           // When < 0, should be only 1 overload type.
   SmallVector<StringRef, 4> counters; // counters for this inst.
   DXILOperationData(const Record *R) {
-    Name = R->getValueAsString("name");
-    DXILOp = R->getValueAsString("dxil_op");
-    DXILOpID = R->getValueAsInt("dxil_opid");
-    DXILClass = R->getValueAsDef("op_class")->getValueAsString("name");
-    Category = R->getValueAsDef("category")->getValueAsString("name");
+    Name = R->getValueAsString("Name");
+    DXILOp = R->getValueAsString("OpName");
+    DXILOpID = R->getValueAsInt("OpCode");
+    DXILClass = R->getValueAsDef("OpClass")->getValueAsString("Name");
+    Category = R->getValueAsDef("OpCategory")->getValueAsString("Name");
 
     if (R->getValue("llvm_intrinsic")) {
       auto *IntrinsicDef = R->getValueAsDef("llvm_intrinsic");
@@ -86,9 +86,9 @@ struct DXILOperationData {
       Intrinsic = DefName.substr(4);
     }
 
-    Doc = R->getValueAsString("doc");
+    Doc = R->getValueAsString("Doc");
 
-    ListInit *ParamList = R->getValueAsListInit("ops");
+    ListInit *ParamList = R->getValueAsListInit("Params");
     OverloadParamIndex = -1;
     for (unsigned I = 0; I < ParamList->size(); ++I) {
       Record *Param = ParamList->getElementAsRecord(I);
@@ -97,8 +97,8 @@ struct DXILOperationData {
       if (CurParam.Kind >= ParameterKind::OVERLOAD)
         OverloadParamIndex = I;
     }
-    OverloadTypes = R->getValueAsString("oload_types");
-    FnAttr = R->getValueAsString("fn_attr");
+    OverloadTypes = R->getValueAsString("OverloadTypes");
+    FnAttr = R->getValueAsString("Attributes");
   }
 };
 } // end anonymous namespace
@@ -122,14 +122,14 @@ static ParameterKind parameterTypeNameToKind(StringRef Name) {
 }
 
 DXILParam::DXILParam(const Record *R) {
-  Name = R->getValueAsString("name");
-  Pos = R->getValueAsInt("pos");
-  Kind = parameterTypeNameToKind(R->getValueAsString("llvm_type"));
-  if (R->getValue("doc"))
-    Doc = R->getValueAsString("doc");
-  IsConst = R->getValueAsBit("is_const");
-  EnumName = R->getValueAsString("enum_name");
-  MaxValue = R->getValueAsInt("max_value");
+  Name = R->getValueAsString("Name");
+  Pos = R->getValueAsInt("Pos");
+  Kind = parameterTypeNameToKind(R->getValueAsString("LLVMType"));
+  if (R->getValue("Doc"))
+    Doc = R->getValueAsString("Doc");
+  IsConst = R->getValueAsBit("IsConstant");
+  EnumName = R->getValueAsString("EnumName");
+  MaxValue = R->getValueAsInt("MaxValue");
 }
 
 static std::string parameterKindToString(ParameterKind Kind) {
@@ -431,7 +431,7 @@ static void emitDXILOperationTable(std::vector<DXILOperationData> &DXILOps,
 }
 
 static void EmitDXILOperation(RecordKeeper &Records, raw_ostream &OS) {
-  std::vector<Record *> Ops = Records.getAllDerivedDefinitions("dxil_op");
+  std::vector<Record *> Ops = Records.getAllDerivedDefinitions("DXILOperation");
   OS << "// Generated code, do not edit.\n";
   OS << "\n";
 

Copy link
Contributor

@bogner bogner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Usage of the acronym DXIL is employed in a freestanding context.

This convention is expected to be followed in DirectX source files
for consistency and readability.
@bharadwajy
Copy link
Contributor Author

I do not have commit access to llvm-project repo. May I request one of the approvers / reviewers to help by pushing the changes? Thanks!

@llvm-beanz
Copy link
Collaborator

I do not have commit access to llvm-project repo. May I request one of the approvers / reviewers to help by pushing the changes? Thanks!

Let's let the buildkite build finish just for posterity, then I'm happy to merge this.

@llvm-beanz
Copy link
Collaborator

The Kite build completed the non-Windows builds and the Windows builders are having trouble keeping up so I'm comfortable merging this.

@llvm-beanz llvm-beanz merged commit 152325d into llvm:main Feb 5, 2024
3 of 4 checks passed
Copy link

github-actions bot commented Feb 5, 2024

@bharadwajy Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may recieve a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

@bharadwajy bharadwajy deleted the dxil-td-camelcase-nfc branch February 5, 2024 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants