[PATCH] D125158: [DirectX backend] Add pass to emit dxil metadata.

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added inline comments.


================
Comment at: llvm/lib/Target/DirectX/DxilEmitMetadata.cpp:24
+}
+ConstantAsMetadata *Uint32ToConstMD(unsigned v, LLVMContext &Ctx) {
+  return ConstantAsMetadata::get(
----------------
Could you space things out more? I'd expect one empty line after each function.


================
Comment at: llvm/lib/Target/DirectX/DxilEmitMetadata.cpp:28
+}
+const StringRef ValVerKey = "dx.valver";
+const unsigned DxilVersionNumFields = 2;
----------------
use StringLiteral


================
Comment at: llvm/lib/Target/DirectX/DxilEmitMetadata.cpp:29
+const StringRef ValVerKey = "dx.valver";
+const unsigned DxilVersionNumFields = 2;
+const unsigned DxilVersionMajorIdx = 0; // DXIL version major.
----------------
nit: we can use `contexpr`


================
Comment at: llvm/test/CodeGen/DirectX/dxil_ver.ll:14
+!2 = !{i32 1, i32 1}
+!3 = !{!"clang version 15.0.0 (https://github.com/llvm/llvm-project 71de12113a0661649ecb2f533fba4a2818a1ad68)"}
----------------
nit: can you add an empty line at the of the file?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125158/new/

https://reviews.llvm.org/D125158