DirectXShaderCompiler icon indicating copy to clipboard operation
DirectXShaderCompiler copied to clipboard

Enable resource directory for DXC.

Open s-perron opened this issue 1 year ago • 3 comments

This change enables the resource directory for DXC. This will allow the HLSL standard headers to be accessed without using the -I option.

s-perron avatar Oct 10 '24 12:10 s-perron

:warning: C/C++ code formatter, clang-format found issues in your code. :warning:

You can test this locally with the following command:
git-clang-format --diff b26fd8099aa435c6a46b10b179549280ba3930b0 a6ebd017c51e5687fd040aaa7a8623c9ee57417f -- tools/clang/lib/Frontend/InitHeaderSearch.cpp tools/clang/tools/dxcompiler/dxcfilesystem.cpp tools/clang/tools/dxcompiler/dxcompilerobj.cpp
View the diff from clang-format here.
diff --git a/tools/clang/lib/Frontend/InitHeaderSearch.cpp b/tools/clang/lib/Frontend/InitHeaderSearch.cpp
index 38612398..fc5627f1 100644
--- a/tools/clang/lib/Frontend/InitHeaderSearch.cpp
+++ b/tools/clang/lib/Frontend/InitHeaderSearch.cpp
@@ -442,12 +442,12 @@ void InitHeaderSearch::AddDefaultIncludePaths(const LangOptions &Lang,
                                               const llvm::Triple &triple,
                                             const HeaderSearchOptions &HSOpts) {
 #if 1 // HLSL Change Starts
-    if (HSOpts.UseBuiltinIncludes) {
-        SmallString<128> P = StringRef(HSOpts.ResourceDir);
-        llvm::sys::path::append(P, "include");
-        AddUnmappedPath(P, Angled, false);
-    }
-    return;
+  if (HSOpts.UseBuiltinIncludes) {
+    SmallString<128> P = StringRef(HSOpts.ResourceDir);
+    llvm::sys::path::append(P, "include");
+    AddUnmappedPath(P, Angled, false);
+  }
+  return;
 #else
   // NB: This code path is going away. All of the logic is moving into the
   // driver which has the information necessary to do target-specific
diff --git a/tools/clang/tools/dxcompiler/dxcfilesystem.cpp b/tools/clang/tools/dxcompiler/dxcfilesystem.cpp
index 19209df0..82633f08 100644
--- a/tools/clang/tools/dxcompiler/dxcfilesystem.cpp
+++ b/tools/clang/tools/dxcompiler/dxcfilesystem.cpp
@@ -397,9 +397,10 @@ public:
       }
     }
     if (compiler.getHeaderSearchOpts().UseBuiltinIncludes) {
-        SmallString<128> P = StringRef(compiler.getHeaderSearchOpts().ResourceDir);
-        llvm::sys::path::append(P, "include");
-        m_searchEntries.emplace_back(Unicode::UTF8ToWideStringOrThrow(P.c_str()));
+      SmallString<128> P =
+          StringRef(compiler.getHeaderSearchOpts().ResourceDir);
+      llvm::sys::path::append(P, "include");
+      m_searchEntries.emplace_back(Unicode::UTF8ToWideStringOrThrow(P.c_str()));
     }
   }
 
diff --git a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp
index 95d2827a..398c91b1 100644
--- a/tools/clang/tools/dxcompiler/dxcompilerobj.cpp
+++ b/tools/clang/tools/dxcompiler/dxcompilerobj.cpp
@@ -1405,8 +1405,8 @@ public:
     // Pick additional arguments.
     clang::HeaderSearchOptions &HSOpts = compiler.getHeaderSearchOpts();
     HSOpts.UseBuiltinIncludes = true;
-    HSOpts.ResourceDir =
-      clang::CompilerInvocation::GetResourcesPath("Hoping this is not needed on any of our platforms", nullptr);
+    HSOpts.ResourceDir = clang::CompilerInvocation::GetResourcesPath(
+        "Hoping this is not needed on any of our platforms", nullptr);
     // Consider: should we force-include '.' if the source file is relative?
     for (const llvm::opt::Arg *A : Opts.Args.filtered(options::OPT_I)) {
       const bool IsFrameworkFalse = false;

  • [ ] Check this box to apply formatting changes to this branch.

github-actions[bot] avatar Oct 10 '24 12:10 github-actions[bot]

Things that still need to be done:

  • [ ] Modify the cmake install to copy the header to the resource directory.
  • [ ] Change the default path to something that makes sense for dxc.
  • [ ] Manually test on a few different platforms to make sure it works.

s-perron avatar Oct 10 '24 12:10 s-perron

@pow2clk @llvm-beanz

Do you have any suggestions on how to implement this? I'm not sure how to proceed. I don't know how to pass information from the driver into the library without changing the public interface.

s-perron avatar Oct 17 '24 13:10 s-perron