Skip to content

[DirectX] Preserve value names in DXILOpLowering. NFC #108089

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 12, 2024
Merged

[DirectX] Preserve value names in DXILOpLowering. NFC #108089

merged 2 commits into from
Sep 12, 2024

Conversation

bogner
Copy link
Contributor

@bogner bogner commented Sep 10, 2024

If the value we're replacing has a name, we might as well preserve it.

@llvmbot
Copy link
Member

llvmbot commented Sep 10, 2024

@llvm/pr-subscribers-backend-directx

Author: Justin Bogner (bogner)

Changes

If the value we're replacing has a name, we might as well preserve it.

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

4 Files Affected:

  • (modified) llvm/lib/Target/DirectX/DXILOpBuilder.cpp (+4-3)
  • (modified) llvm/lib/Target/DirectX/DXILOpBuilder.h (+2-1)
  • (modified) llvm/lib/Target/DirectX/DXILOpLowering.cpp (+10-9)
  • (modified) llvm/test/CodeGen/DirectX/CreateHandleFromBinding.ll (+6-6)
diff --git a/llvm/lib/Target/DirectX/DXILOpBuilder.cpp b/llvm/lib/Target/DirectX/DXILOpBuilder.cpp
index 3b2a5f5061eb83..7719d6b1079110 100644
--- a/llvm/lib/Target/DirectX/DXILOpBuilder.cpp
+++ b/llvm/lib/Target/DirectX/DXILOpBuilder.cpp
@@ -386,6 +386,7 @@ static Error makeOpError(dxil::OpCode OpCode, Twine Msg) {
 
 Expected<CallInst *> DXILOpBuilder::tryCreateOp(dxil::OpCode OpCode,
                                                 ArrayRef<Value *> Args,
+                                                const Twine &Name,
                                                 Type *RetTy) {
   const OpCodeProperty *Prop = getOpCodeProperty(OpCode);
 
@@ -451,12 +452,12 @@ Expected<CallInst *> DXILOpBuilder::tryCreateOp(dxil::OpCode OpCode,
   OpArgs.push_back(IRB.getInt32(llvm::to_underlying(OpCode)));
   OpArgs.append(Args.begin(), Args.end());
 
-  return IRB.CreateCall(DXILFn, OpArgs);
+  return IRB.CreateCall(DXILFn, OpArgs, Name);
 }
 
 CallInst *DXILOpBuilder::createOp(dxil::OpCode OpCode, ArrayRef<Value *> Args,
-                                  Type *RetTy) {
-  Expected<CallInst *> Result = tryCreateOp(OpCode, Args, RetTy);
+                                  const Twine &Name, Type *RetTy) {
+  Expected<CallInst *> Result = tryCreateOp(OpCode, Args, Name, RetTy);
   if (Error E = Result.takeError())
     llvm_unreachable("Invalid arguments for operation");
   return *Result;
diff --git a/llvm/lib/Target/DirectX/DXILOpBuilder.h b/llvm/lib/Target/DirectX/DXILOpBuilder.h
index a68f0c43f67afb..037ae3822cfb90 100644
--- a/llvm/lib/Target/DirectX/DXILOpBuilder.h
+++ b/llvm/lib/Target/DirectX/DXILOpBuilder.h
@@ -39,11 +39,12 @@ class DXILOpBuilder {
   /// Create a call instruction for the given DXIL op. The arguments
   /// must be valid for an overload of the operation.
   CallInst *createOp(dxil::OpCode Op, ArrayRef<Value *> Args,
-                     Type *RetTy = nullptr);
+                     const Twine &Name = "", Type *RetTy = nullptr);
 
   /// Try to create a call instruction for the given DXIL op. Fails if the
   /// overload is invalid.
   Expected<CallInst *> tryCreateOp(dxil::OpCode Op, ArrayRef<Value *> Args,
+                                   const Twine &Name = "",
                                    Type *RetTy = nullptr);
 
   /// Get a `%dx.types.ResRet` type with the given element type.
diff --git a/llvm/lib/Target/DirectX/DXILOpLowering.cpp b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
index f968cab1dccf1e..b93ba28048ed41 100644
--- a/llvm/lib/Target/DirectX/DXILOpLowering.cpp
+++ b/llvm/lib/Target/DirectX/DXILOpLowering.cpp
@@ -118,7 +118,7 @@ class OpLowerer {
         Args.append(CI->arg_begin(), CI->arg_end());
 
       Expected<CallInst *> OpCall =
-          OpBuilder.tryCreateOp(DXILOp, Args, F.getReturnType());
+          OpBuilder.tryCreateOp(DXILOp, Args, CI->getName(), F.getReturnType());
       if (Error E = OpCall.takeError())
         return E;
 
@@ -198,7 +198,7 @@ class OpLowerer {
           ConstantInt::get(Int32Ty, Binding.RecordID), CI->getArgOperand(3),
           CI->getArgOperand(4)};
       Expected<CallInst *> OpCall =
-          OpBuilder.tryCreateOp(OpCode::CreateHandle, Args);
+          OpBuilder.tryCreateOp(OpCode::CreateHandle, Args, CI->getName());
       if (Error E = OpCall.takeError())
         return E;
 
@@ -233,15 +233,16 @@ class OpLowerer {
           Binding.LowerBound, UpperBound, Binding.Space, RI.getResourceClass());
       std::array<Value *, 3> BindArgs{ResBind, CI->getArgOperand(3),
                                       CI->getArgOperand(4)};
-      Expected<CallInst *> OpBind =
-          OpBuilder.tryCreateOp(OpCode::CreateHandleFromBinding, BindArgs);
+      Expected<CallInst *> OpBind = OpBuilder.tryCreateOp(
+          OpCode::CreateHandleFromBinding, BindArgs, CI->getName());
       if (Error E = OpBind.takeError())
         return E;
 
       std::array<Value *, 2> AnnotateArgs{
           *OpBind, OpBuilder.getResProps(Props.first, Props.second)};
-      Expected<CallInst *> OpAnnotate =
-          OpBuilder.tryCreateOp(OpCode::AnnotateHandle, AnnotateArgs);
+      Expected<CallInst *> OpAnnotate = OpBuilder.tryCreateOp(
+          OpCode::AnnotateHandle, AnnotateArgs,
+          CI->hasName() ? CI->getName() + "_annot" : Twine());
       if (Error E = OpAnnotate.takeError())
         return E;
 
@@ -361,8 +362,8 @@ class OpLowerer {
       Type *NewRetTy = OpBuilder.getResRetType(CI->getType()->getScalarType());
 
       std::array<Value *, 3> Args{Handle, Index0, Index1};
-      Expected<CallInst *> OpCall =
-          OpBuilder.tryCreateOp(OpCode::BufferLoad, Args, NewRetTy);
+      Expected<CallInst *> OpCall = OpBuilder.tryCreateOp(
+          OpCode::BufferLoad, Args, CI->getName(), NewRetTy);
       if (Error E = OpCall.takeError())
         return E;
       if (Error E = replaceResRetUses(CI, *OpCall))
@@ -405,7 +406,7 @@ class OpLowerer {
       std::array<Value *, 8> Args{Handle, Index0, Index1, Data0,
                                   Data1,  Data2,  Data3,  Mask};
       Expected<CallInst *> OpCall =
-          OpBuilder.tryCreateOp(OpCode::BufferStore, Args);
+          OpBuilder.tryCreateOp(OpCode::BufferStore, Args, CI->getName());
       if (Error E = OpCall.takeError())
         return E;
 
diff --git a/llvm/test/CodeGen/DirectX/CreateHandleFromBinding.ll b/llvm/test/CodeGen/DirectX/CreateHandleFromBinding.ll
index d0c80c018b8d7e..dbdd2e61df7a3b 100644
--- a/llvm/test/CodeGen/DirectX/CreateHandleFromBinding.ll
+++ b/llvm/test/CodeGen/DirectX/CreateHandleFromBinding.ll
@@ -19,14 +19,14 @@ define void @test_bindings() {
   %typed0 = call target("dx.TypedBuffer", <4 x float>, 1, 0, 0)
               @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_1_0_0(
                   i32 3, i32 5, i32 1, i32 4, i1 false)
-  ; CHECK: [[BUF0:%[0-9]*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 5, i32 5, i32 3, i8 1 }, i32 4, i1 false)
+  ; CHECK: [[BUF0:%.*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 5, i32 5, i32 3, i8 1 }, i32 4, i1 false)
   ; CHECK: call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BUF0]], %dx.types.ResourceProperties { i32 4106, i32 1033 })
 
   ; RWBuffer<int> Buf : register(u7, space2)
   %typed1 = call target("dx.TypedBuffer", i32, 1, 0, 1)
       @llvm.dx.handle.fromBinding.tdx.TypedBuffer_i32_1_0_0t(
           i32 2, i32 7, i32 1, i32 6, i1 false)
-  ; CHECK: [[BUF1:%[0-9]*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 7, i32 7, i32 2, i8 1 }, i32 6, i1 false)
+  ; CHECK: [[BUF1:%.*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 7, i32 7, i32 2, i8 1 }, i32 6, i1 false)
   ; CHECK: call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BUF1]], %dx.types.ResourceProperties { i32 4106, i32 260 })
 
   ; Buffer<uint4> Buf[24] : register(t3, space5)
@@ -35,7 +35,7 @@ define void @test_bindings() {
   %typed2 = call target("dx.TypedBuffer", <4 x i32>, 0, 0, 0)
       @llvm.dx.handle.fromBinding.tdx.TypedBuffer_i32_0_0_0t(
           i32 5, i32 3, i32 24, i32 7, i1 false)
-  ; CHECK: [[BUF2:%[0-9]*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 3, i32 26, i32 5, i8 0 }, i32 7, i1 false)
+  ; CHECK: [[BUF2:%.*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 3, i32 26, i32 5, i8 0 }, i32 7, i1 false)
   ; CHECK: call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BUF2]], %dx.types.ResourceProperties { i32 10, i32 1029 })
 
   ; struct S { float4 a; uint4 b; };
@@ -43,14 +43,14 @@ define void @test_bindings() {
   %struct0 = call target("dx.RawBuffer", {<4 x float>, <4 x i32>}, 0, 0)
       @llvm.dx.handle.fromBinding.tdx.RawBuffer_sl_v4f32v4i32s_0_0t(
           i32 4, i32 2, i32 1, i32 10, i1 true)
-  ; CHECK: [[BUF3:%[0-9]*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 2, i32 2, i32 4, i8 0 }, i32 10, i1 true)
+  ; CHECK: [[BUF3:%.*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 2, i32 2, i32 4, i8 0 }, i32 10, i1 true)
   ; CHECK: = call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BUF3]], %dx.types.ResourceProperties { i32 1036, i32 32 })
 
   ; ByteAddressBuffer Buf : register(t8, space1)
   %byteaddr0 = call target("dx.RawBuffer", i8, 0, 0)
       @llvm.dx.handle.fromBinding.tdx.RawBuffer_i8_0_0t(
           i32 1, i32 8, i32 1, i32 12, i1 false)
-  ; CHECK: [[BUF4:%[0-9]*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 8, i32 8, i32 1, i8 0 }, i32 12, i1 false)
+  ; CHECK: [[BUF4:%.*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 8, i32 8, i32 1, i8 0 }, i32 12, i1 false)
   ; CHECK: call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BUF4]], %dx.types.ResourceProperties { i32 11, i32 0 })
 
   ; Buffer<float4> Buf[] : register(t0)
@@ -59,7 +59,7 @@ define void @test_bindings() {
   %typed3 = call target("dx.TypedBuffer", <4 x float>, 0, 0, 0)
       @llvm.dx.handle.fromBinding.tdx.TypedBuffer_v4f32_0_0_0t(
           i32 0, i32 0, i32 -1, i32 %typed3_ix, i1 false)
-  ; CHECK: [[BUF5:%[0-9]*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 0, i32 -1, i32 0, i8 0 }, i32 %typed3_ix, i1 false)
+  ; CHECK: [[BUF5:%.*]] = call %dx.types.Handle @dx.op.createHandleFromBinding(i32 218, %dx.types.ResBind { i32 0, i32 -1, i32 0, i8 0 }, i32 %typed3_ix, i1 false)
   ; CHECK: call %dx.types.Handle @dx.op.annotateHandle(i32 217, %dx.types.Handle [[BUF5]], %dx.types.ResourceProperties { i32 10, i32 1033 })
 
   ret void

coopp
coopp approved these changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@coopp coopp left a comment

Choose a reason for hiding this comment

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

Yep.. names looked preserved. So the CHECK: tests are looking for anything now when before they looked for any number?

@bogner
Copy link
Contributor Author

bogner commented Sep 11, 2024

Yep.. names looked preserved. So the CHECK: tests are looking for anything now when before they looked for any number?

Correct. I don't think we can actually check that the names match the input names because there are configs where all value names get dropped.

@bogner bogner force-pushed the 2024-09-10-dxilop-valuenames branch from ff92ea3 to 8e512d7 Compare September 11, 2024 23:26
@bogner bogner merged commit 3d12901 into llvm:main Sep 12, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coopp coopp coopp approved these changes

@python3kgae python3kgae python3kgae approved these changes

Assignees
No one assigned
Labels
backend:DirectX
Projects
HLSL Support
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bogner @llvmbot @coopp @python3kgae