Skip to content

Conversation

@bogner
Copy link
Contributor

@bogner bogner commented Aug 14, 2024

DXIL operations that only have one signature behave one of two ways - either
they are always suffixed with a type like dx.op.ThreadId.i32 and hence have
exactly one overload, or they're never suffixed like dx.op.CreateHandle and
hence have zero overloads.

Update DXIL.td for operations that have one overload and remove the hack in the
builder that was adjusting names for unoverloaded ops.

bogner added 2 commits August 15, 2024 00:27
Created using spr 1.3.5-bogner

[skip ci]
Created using spr 1.3.5-bogner
@llvmbot
Copy link
Member

llvmbot commented Aug 14, 2024

@llvm/pr-subscribers-backend-directx

Author: Justin Bogner (bogner)

Changes

DXIL operations that only have one signature behave one of two ways - either
they are always suffixed with a type like dx.op.ThreadId.i32 and hence have
exactly one overload, or they're never suffixed like dx.op.CreateHandle and
hence have zero overloads.

Update DXIL.td for operations that have one overload and remove the hack in the
builder that was adjusting names for unoverloaded ops.


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

2 Files Affected:

  • (modified) llvm/lib/Target/DirectX/DXIL.td (+11-7)
  • (modified) llvm/lib/Target/DirectX/DXILOpBuilder.cpp (+4-5)
diff --git a/llvm/lib/Target/DirectX/DXIL.td b/llvm/lib/Target/DirectX/DXIL.td
index 67015cff78a79..60185c20606b2 100644
--- a/llvm/lib/Target/DirectX/DXIL.td
+++ b/llvm/lib/Target/DirectX/DXIL.td
@@ -671,8 +671,9 @@ def Dot4 :  DXILOp<56, dot4> {
 def ThreadId :  DXILOp<93, threadId> {
   let Doc = "Reads the thread ID";
   let LLVMIntrinsic = int_dx_thread_id;
-  let arguments = [i32Ty];
-  let result = i32Ty;
+  let arguments = [overloadTy];
+  let result = overloadTy;
+  let overloads = [Overloads<DXIL1_0, [i32Ty]>];
   let stages = [Stages<DXIL1_0, [compute, mesh, amplification, node]>];
   let attributes = [Attributes<DXIL1_0, [ReadNone]>];
 }
@@ -680,8 +681,9 @@ def ThreadId :  DXILOp<93, threadId> {
 def GroupId :  DXILOp<94, groupId> {
   let Doc = "Reads the group ID (SV_GroupID)";
   let LLVMIntrinsic = int_dx_group_id;
-  let arguments = [i32Ty];
-  let result = i32Ty;
+  let arguments = [overloadTy];
+  let result = overloadTy;
+  let overloads = [Overloads<DXIL1_0, [i32Ty]>];
   let stages = [Stages<DXIL1_0, [compute, mesh, amplification, node]>];
   let attributes = [Attributes<DXIL1_0, [ReadNone]>];
 }
@@ -689,8 +691,9 @@ def GroupId :  DXILOp<94, groupId> {
 def ThreadIdInGroup :  DXILOp<95, threadIdInGroup> {
   let Doc = "Reads the thread ID within the group  (SV_GroupThreadID)";
   let LLVMIntrinsic = int_dx_thread_id_in_group;
-  let arguments = [i32Ty];
-  let result = i32Ty;
+  let arguments = [overloadTy];
+  let result = overloadTy;
+  let overloads = [Overloads<DXIL1_0, [i32Ty]>];
   let stages = [Stages<DXIL1_0, [compute, mesh, amplification, node]>];
   let attributes = [Attributes<DXIL1_0, [ReadNone]>];
 }
@@ -699,7 +702,8 @@ def FlattenedThreadIdInGroup :  DXILOp<96, flattenedThreadIdInGroup> {
   let Doc = "Provides a flattened index for a given thread within a given "
             "group (SV_GroupIndex)";
   let LLVMIntrinsic = int_dx_flattened_thread_id_in_group;
-  let result = i32Ty;
+  let result = overloadTy;
+  let overloads = [Overloads<DXIL1_0, [i32Ty]>];
   let stages = [Stages<DXIL1_0, [compute, mesh, amplification, node]>];
   let attributes = [Attributes<DXIL1_0, [ReadNone]>];
 }
diff --git a/llvm/lib/Target/DirectX/DXILOpBuilder.cpp b/llvm/lib/Target/DirectX/DXILOpBuilder.cpp
index 91e6931b3f788..0e2b4601112b5 100644
--- a/llvm/lib/Target/DirectX/DXILOpBuilder.cpp
+++ b/llvm/lib/Target/DirectX/DXILOpBuilder.cpp
@@ -87,6 +87,9 @@ static const char *getOverloadTypeName(OverloadKind Kind) {
 }
 
 static OverloadKind getOverloadKind(Type *Ty) {
+  if (!Ty)
+    return OverloadKind::VOID;
+
   Type::TypeID T = Ty->getTypeID();
   switch (T) {
   case Type::VoidTyID:
@@ -379,11 +382,7 @@ Expected<CallInst *> DXILOpBuilder::tryCreateOp(dxil::OpCode OpCode,
 
   uint16_t ValidTyMask = Prop->Overloads[*OlIndexOrErr].ValidTys;
 
-  // If we don't have an overload type, use the function's return type. This is
-  // a bit of a hack, but it's necessary to get the type suffix on unoverloaded
-  // DXIL ops correct, like `dx.op.threadId.i32`.
-  OverloadKind Kind =
-      getOverloadKind(OverloadTy ? OverloadTy : DXILOpFT->getReturnType());
+  OverloadKind Kind = getOverloadKind(OverloadTy);
 
   // Check if the operation supports overload types and OverloadTy is valid
   // per the specified types for the operation

bogner added a commit to bogner/llvm-project that referenced this pull request Aug 14, 2024
DXIL operations that only have one signature behave one of two ways - either
they are always suffixed with a type like `dx.op.ThreadId.i32` and hence have
exactly one overload, or they're never suffixed like `dx.op.CreateHandle` and
hence have zero overloads.

Update DXIL.td for operations that have one overload and remove the hack in the
builder that was adjusting names for unoverloaded ops.

Pull Request: llvm#104246
Created using spr 1.3.5-bogner
@bogner bogner changed the base branch from users/bogner/sprmain.directx-differentiate-between-01-overloads-in-the-opbuilder-nfc to main August 19, 2024 16:55
@bogner bogner merged commit 7d60f46 into main Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants