[DirectX backend] remove hlsl.* FnAttr after use.

[python3kgae]

Summary

Function attribute like hlsl.shader is not needed after DXILTranslateMetadata pass.
And it should be removed for DXIL.

So remove it once the information is collect.

[python3kgae]

Fix format.

[python3kgae]

Remove hlsl attr in DXILTranslateMetadata.

[beanz]

llvm/lib/Target/DirectX/DXILMetadata.cpp

99

I'm not sure I understand this refactoring... It looks like you're trying to avoid looking up the attribute more than once, but it means you're keeping track of the Attribute and passing it around through layers... I'm not convinced that is beneficial.

[python3kgae]

llvm/lib/Target/DirectX/DXILMetadata.cpp

99

How about send the Entry shader kind for library directly?
Then we can remove the if (ShaderKind == Triple::EnvironmentType::Library) and no extra Attribute needs to be sent down.

[python3kgae]

Avoid send EntryAttr as input parameter for EntryProps constructor.

llvm/lib/Target/DirectX/DXILMetadata.cpp

struct EntryProps {
   Triple::EnvironmentType ShaderKind;
   // FIXME: support more shader profiles.
   // See https://github.com/llvm/llvm-project/issues/57927.
   struct {
     unsigned NumThreads[3];
   } CS;
 
   EntryProps(Function &F, Triple::EnvironmentType ModuleShaderKind)
       : ShaderKind(ModuleShaderKind) {
 
-    if (ShaderKind == Triple::EnvironmentType::Library) {
-      Attribute EntryAttr = F.getFnAttribute("hlsl.shader");
-      StringRef EntryProfile = EntryAttr.getValueAsString();
-      Triple T("", "", "", EntryProfile);
-      ShaderKind = T.getEnvironment();
-    }
-
     if (ShaderKind == Triple::EnvironmentType::Compute) {
       auto NumThreadsStr =
           F.getFnAttribute("hlsl.numthreads").getValueAsString();
       SmallVector<StringRef> NumThreads;
       NumThreadsStr.split(NumThreads, ',');
       assert(NumThreads.size() == 3 && "invalid numthreads");
       auto Zip =
           llvm::zip(NumThreads, MutableArrayRef<unsigned>(CS.NumThreads));
static MDTuple *emitDxilEntryPointTuple(Function *Fn, const std::string &Name,
     MDVals[3] = Resources;
     MDVals[4] = Properties;
     return MDNode::get(Ctx, MDVals);
   }
 };
 } // namespace
 
 void dxil::createEntryMD(Module &M, const uint64_t ShaderFlags) {
-  SmallVector<Function *> EntryList;
+  SmallVector<std::pair<Function *, Attribute>> EntryList;
   for (auto &F : M.functions()) {
     if (!F.hasFnAttribute("hlsl.shader"))
       continue;
-    EntryList.emplace_back(&F);
+    EntryList.emplace_back(std::make_pair(&F, F.getFnAttribute("hlsl.shader")));
   }
 
   auto &Ctx = M.getContext();
   // FIXME: generate metadata for resource.
   // See https://github.com/llvm/llvm-project/issues/57926.
   MDTuple *MDResources = nullptr;
   if (auto *NamedResources = M.getNamedMetadata("dx.resources"))
     MDResources = dyn_cast<MDTuple>(NamedResources->getOperand(0));
 
   std::vector<MDNode *> Entries;
   Triple T = Triple(M.getTargetTriple());
   switch (T.getEnvironment()) {
   case Triple::EnvironmentType::Library: {
     // Add empty entry to put resource metadata.
     MDTuple *EmptyEntry =
         EntryMD::emitEmptyEntryForLib(MDResources, ShaderFlags, Ctx);
     Entries.emplace_back(EmptyEntry);
 
-    for (Function *Entry : EntryList) {
-      EntryMD MD(*Entry, T.getEnvironment());
+    for (auto &It : EntryList) {
+      StringRef EntryProfile = It.second.getValueAsString();
+      Triple T("", "", "", EntryProfile);
+      EntryMD MD(*It.first, T.getEnvironment());
       Entries.emplace_back(MD.emitEntryTupleForLib(0));
     }
   } break;
   case Triple::EnvironmentType::Compute:
   case Triple::EnvironmentType::Amplification:
   case Triple::EnvironmentType::Mesh:
   case Triple::EnvironmentType::Vertex:
   case Triple::EnvironmentType::Hull:
   case Triple::EnvironmentType::Domain:
   case Triple::EnvironmentType::Geometry:
   case Triple::EnvironmentType::Pixel: {
     assert(EntryList.size() == 1 &&
            "non-lib profiles should only have one entry");
-    EntryMD MD(*EntryList.front(), T.getEnvironment());
+    EntryMD MD(*EntryList.front().first, T.getEnvironment());
     Entries.emplace_back(MD.emitEntryTuple(MDResources, ShaderFlags));
   } break;
   default:
     assert(0 && "invalid profile");
     break;
   }
 
   NamedMDNode *EntryPointsNamedMD =
       M.getOrInsertNamedMetadata("dx.entryPoints");
   for (auto *Entry : Entries)
     EntryPointsNamedMD->addOperand(Entry);
 }

llvm/lib/Target/DirectX/DXILTranslateMetadata.cpp

public:
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.setPreservesAll();
     AU.addRequired<DXILResourceWrapper>();
     AU.addRequired<ShaderFlagsAnalysisWrapper>();
   }
 
   bool runOnModule(Module &M) override;
+private:
+  void removeUnusedAttrs(Module &M);
 };
 
 } // namespace
 
+void DXILTranslateMetadata::removeUnusedAttrs(Module &M) {
+  const StringRef UnusedAttrs[] = {"hlsl.shader", "hlsl.numthreads"};
+  for (auto &F : M.functions()) {
+    for (auto &Attr : UnusedAttrs) {
+      if (F.hasFnAttribute(Attr))
+        F.removeFnAttr(Attr);
+    }
+  }
+}
+
 bool DXILTranslateMetadata::runOnModule(Module &M) {
 
   dxil::ValidatorVersionMD ValVerMD(M);
   if (ValVerMD.isEmpty())
     ValVerMD.update(VersionTuple(1, 0));
   dxil::createShaderModelMD(M);
 
   const dxil::Resources &Res =
       getAnalysis<DXILResourceWrapper>().getDXILResource();
   Res.write(M);
 
   const uint64_t Flags =
       (uint64_t)(getAnalysis<ShaderFlagsAnalysisWrapper>().getShaderFlags());
   dxil::createEntryMD(M, Flags);
 
-  return false;
+  removeUnusedAttrs(M);
+  return true;
 }
 
 char DXILTranslateMetadata::ID = 0;
 
 ModulePass *llvm::createDXILTranslateMetadataPass() {
   return new DXILTranslateMetadata();
 }
 
 INITIALIZE_PASS_BEGIN(DXILTranslateMetadata, "dxil-metadata-emit",
                       "DXIL Metadata Emit", false, false)
 INITIALIZE_PASS_DEPENDENCY(DXILResourceWrapper)
 INITIALIZE_PASS_DEPENDENCY(ShaderFlagsAnalysisWrapper)
 INITIALIZE_PASS_END(DXILTranslateMetadata, "dxil-metadata-emit",
                     "DXIL Metadata Emit", false, false)

llvm/test/CodeGen/DirectX/hlsl_attr.ll

+; RUN: opt -S -dxil-metadata-emit < %s | FileCheck %s
+
+
+; CHECK-LABEL:define void @CSMain()
+
+; Make sure hlsl.* is removed from function attribute.
+; CHECK-NOT:"hlsl.numthreads"
+; CHECK-NOT:"hlsl.shader"
+
+; CHECK:!dx.shaderModel = !{![[ShaderModel:[0-9]+]]}
+; CHECK:!dx.entryPoints = !{![[ENTRY:[0-9]+]]}
+; CHECK:![[ShaderModel]] = !{!"cs", i32 6, i32 0}
+; CHECK:![[RES:[0-9]+]] = !{null, ![[UAV:[0-9]+]], null, null}
+; CHECK:![[UAV]] = !{![[SRCBUF:[0-9]+]], ![[DSTBUF:[0-9]+]]}
+; CHECK:![[SRCBUF]] = !{i32 0, ptr @"?srcBuffer@@3V?$RWBuffer@T?$__vector@H$01@__clang@@@hlsl@@A", !"", i32 0, i32 1, i32 1, i32 10, i1 false, i1 false, i1 false, null}
+; CHECK:![[DSTBUF]] = !{i32 1, ptr @"?dstBuffer@@3V?$RWBuffer@T?$__vector@H$01@__clang@@@hlsl@@A", !"", i32 0, i32 0, i32 1, i32 10, i1 false, i1 false, i1 false, null}
+; CHECK:![[ENTRY]] = !{ptr @CSMain, !"CSMain", null, ![[RES]], ![[EXTRA:[0-9]+]]}
+; CHECK:![[EXTRA]] = !{i32 4, ![[NUMTHREADS:[0-9]+]]}
+; CHECK:![[NUMTHREADS]] = !{i32 1024, i32 1, i32 1}
+
+target datalayout = "e-m:e-p:32:32-i1:32-i8:8-i16:16-i32:32-i64:64-f16:16-f32:32-f64:64-n8:16:32:64"
+target triple = "dxil-unknown-shadermodel6.0-compute"
+
+%"class.hlsl::RWBuffer" = type { ptr }
+%dx.types.Handle = type { ptr }
+%dx.types.ResRet.i32 = type { i32, i32, i32, i32, i32 }
+
+@"?srcBuffer@@3V?$RWBuffer@T?$__vector@H$01@__clang@@@hlsl@@A" = local_unnamed_addr global %"class.hlsl::RWBuffer" zeroinitializer, align 4
+@"?dstBuffer@@3V?$RWBuffer@T?$__vector@H$01@__clang@@@hlsl@@A" = local_unnamed_addr global %"class.hlsl::RWBuffer" zeroinitializer, align 4
+
+; Function Attrs: mustprogress nofree nounwind willreturn memory(readwrite, inaccessiblemem: read)
+define void @CSMain() local_unnamed_addr #0 {
+entry:
+  %0 = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 1, i32 0, i1 false)
+  %1 = call %dx.types.Handle @dx.op.createHandle(i32 57, i8 1, i32 0, i32 1, i1 false)
+  %2 = call i32 @dx.op.threadId.i32(i32 93, i32 0)
+  %3 = call %dx.types.ResRet.i32 @dx.op.bufferLoad.i32(i32 68, %dx.types.Handle %1, i32 %2, i32 poison)
+  %4 = extractvalue %dx.types.ResRet.i32 %3, 0
+  %5 = extractvalue %dx.types.ResRet.i32 %3, 1
+  %add.i.i0 = add i32 %4, 10
+  %add.i.i1 = add i32 %5, 10
+  call void @dx.op.bufferStore.i32(i32 69, %dx.types.Handle %0, i32 %2, i32 poison, i32 %add.i.i0, i32 %add.i.i1, i32 %add.i.i0, i32 %add.i.i0, i8 15)
+  ret void
+}
+
+declare %dx.types.Handle @dx.op.createHandle(i32 %0, i8 %1, i32 %2, i32 %3, i1 %4)
+
+declare %dx.types.ResRet.i32 @dx.op.bufferLoad.i32(i32 %0, %dx.types.Handle %1, i32 %2, i32 %3)
+
+declare void @dx.op.bufferStore.i32(i32 %0, %dx.types.Handle %1, i32 %2, i32 %3, i32 %4, i32 %5, i32 %6, i32 %7, i8 %8)
+
+declare i32 @dx.op.threadId.i32(i32 %0, i32 %1)
+
+attributes #0 = { mustprogress nofree nounwind willreturn memory(readwrite, inaccessiblemem: read) "frame-pointer"="all" "hlsl.numthreads"="1024,1,1" "hlsl.shader"="compute" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
+
+!hlsl.uavs = !{!0, !1}
+!llvm.module.flags = !{!2, !3}
+!dx.valver = !{!4}
+!llvm.ident = !{!5}
+
+!0 = !{ptr @"?srcBuffer@@3V?$RWBuffer@T?$__vector@H$01@__clang@@@hlsl@@A", !"RWBuffer<int2>", i32 10, i32 1, i32 0}
+!1 = !{ptr @"?dstBuffer@@3V?$RWBuffer@T?$__vector@H$01@__clang@@@hlsl@@A", !"RWBuffer<int2>", i32 10, i32 0, i32 0}
+!2 = !{i32 1, !"wchar_size", i32 4}
+!3 = !{i32 7, !"frame-pointer", i32 2}
+!4 = !{i32 1, i32 1}
+!5 = !{!"clang version 16.0.0 (https://github.com/llvm/llvm-project.git 9641897a6b7b727fd5139d7bf92cb6b027037193)"}