Remove IncludeHandler. (shader-slang#1505)

nvAPI -> NVAPI
nvAPIPath -> nvapiPath
DxcIncludeHandler don't reference count.
nv-api-path -> nvapi-path

Co-authored-by: Tim Foley <tfoleyNV@users.noreply.github.com>

Author: jsmall-zzz

docs/target-compatibility.md

@@ -192,9 +192,9 @@ void RWByteAddressBuffer::InterlockedAddI64(uint byteAddress, int64_t valueToAdd
 void RWByteAddressBuffer::InterlockedAddI64(uint byteAddress, int64_t valueToAdd);
 ```
 
-On HLSL based targets this functionality is achieved using [nvAPI](https://developer.nvidia.com/nvapi) based functionality. Therefore for the feature to work you must have nvAPI installed on your system. Then the 'prelude' functionality allows via the API for an include (or the text) of the relevent files. To see how to do this in practice look at the function `setSessionDefaultPrelude`. This makes the prelude for HLSL hold an include to the *absolute* path to the required include file `nvHLSLExtns.h`. As an absolute path is used, it means other includes that includes, look in the correct place without having to set up special include paths. 
+On HLSL based targets this functionality is achieved using [NVAPI](https://developer.nvidia.com/nvapi) based functionality. Therefore for the feature to work you must have NVAPI installed on your system. Then the 'prelude' functionality allows via the API for an include (or the text) of the relevent files. To see how to do this in practice look at the function `setSessionDefaultPrelude`. This makes the prelude for HLSL hold an include to the *absolute* path to the required include file `nvHLSLExtns.h`. As an absolute path is used, it means other includes that includes, look in the correct place without having to set up special include paths. 
 
-To use nvAPI it is nessary to specify a unordered access views (UAV) based 'u' register that will be used to communicate with nvAPI. Note! Slang does not do any special handling around this, it will be necessary for application code to ensure the UAV is either guarenteed to not collide with what Slang assigns, or it's specified (but not used) in the Slang source. The u register number has to be specified also to the nvAPI runtime library. 
+To use NVAPI it is nessary to specify a unordered access views (UAV) based 'u' register that will be used to communicate with NVAPI. Note! Slang does not do any special handling around this, it will be necessary for application code to ensure the UAV is either guarenteed to not collide with what Slang assigns, or it's specified (but not used) in the Slang source. The u register number has to be specified also to the NVAPI runtime library. 
 
 On Vulkan, for float the [`GL_EXT_shader_atomic_float`](https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VK_EXT_shader_atomic_float.html) extension is required. For int64 the [`GL_EXT_shader_atomic_int64`](https://raw.githubusercontent.com/KhronosGroup/GLSL/master/extensions/ext/GL_EXT_shader_atomic_int64.txt) extension is required.
 

source/core/slang-test-tool-util.cpp

@@ -100,10 +100,10 @@ static SlangResult _addCUDAPrelude(const String& parentPath, slang::IGlobalSessi
         }
     }
     // If the nvAPI path is set, and we find nvHLSLExtns.h, put that in the HLSL prelude
-    if (info.nvAPIPath)
+    if (info.nvapiPath)
     {
         String includePath;
-        if (SLANG_SUCCEEDED(_calcIncludePath(info.nvAPIPath, "nvHLSLExtns.h", includePath)))
+        if (SLANG_SUCCEEDED(_calcIncludePath(info.nvapiPath, "nvHLSLExtns.h", includePath)))
         {
             StringBuilder buf;
 

source/core/slang-test-tool-util.h

@@ -39,7 +39,7 @@ struct TestToolUtil
     struct PreludeInfo
     {
         const char* exePath = nullptr;
-        const char* nvAPIPath = nullptr;
+        const char* nvapiPath = nullptr;
     };
 
     typedef SlangResult(*InnerMainFunc)(Slang::StdWriters* stdWriters, SlangSession* session, int argc, const char*const* argv);

source/slang/hlsl.meta.slang

@@ -196,8 +196,8 @@ ${{{{
 
     // float32 and int64 atomic support. This is a Slang specific extension, it uses
     // GL_EXT_shader_atomic_float on vk
-    // NvAPI support on DX
-    // NOTE! To use this feature on HLSL, the shader needs to include 'nvHLSLExtns.h' from the NvAPI SDK
+    // NVAPI support on DX
+    // NOTE! To use this feature on HLSL, the shader needs to include 'nvHLSLExtns.h' from the NVAPI SDK
     //
 
     // Fp32 

source/slang/slang-check-decl.cpp

@@ -3520,25 +3520,6 @@ namespace Slang
         return subst;
     }
 
-#if 0
-    // For simplicity we will make having a definition of a function include having a body or a target intrinsics defined.
-    // It may be useful to add other modifiers to mark as having body - for example perhaps
-    // any target intrinsic modifier (like SPIR-V version) should be included.
-    //
-    // Note that not having this check around TargetIntrinsicModifier can lead to a crash in the compiler
-    // with a definition, followed by a declaration with a target intrinsic.
-    // That this doesn't appear to be the case with other modifiers.
-    // TODO: 
-    // We may want to be able to add target intrinsics with other declarations, that being the case this logic
-    // would need to change.
-    // We might also want are more precise error that pointed out the actually problem - because strictly speaking
-    // having a target intrinsic isn't a 'body'.
-    bool _isDefinition(FuncDecl* decl)
-    {
-        return decl->body || decl->hasModifier<TargetIntrinsicModifier>();
-    }
-#endif
-
     typedef Dictionary<Name*, CallableDecl*> TargetDeclDictionary;
 
     static void _addTargetModifiers(CallableDecl* decl, TargetDeclDictionary& ioDict)

source/slang/slang-dxc-support.cpp

@@ -50,7 +50,7 @@ namespace Slang
     static const Guid IID_IDxcIncludeHandler = { 0x7f61fc7d, 0x950d, 0x467f, { 0x3c, 0x02, 0xfb, 0x49, 0x18, 0x7c } };
     static const Guid IID_IUnknown = SLANG_UUID_ISlangUnknown;
 
-    class DxcIncludeHandler : public IDxcIncludeHandler, public RefObject
+    class DxcIncludeHandler : public IDxcIncludeHandler
     {
     public:
         // Implement IUnknown
@@ -59,14 +59,13 @@ namespace Slang
             ISlangUnknown* intf = getInterface(reinterpret_cast<const Guid&>(uuid)); 
             if (intf) 
             { 
-                addReference(); 
                 *out = intf; 
                 return SLANG_OK;
             } 
             return SLANG_E_NO_INTERFACE;
         }
-        SLANG_NO_THROW ULONG SLANG_MCALL AddRef() SLANG_OVERRIDE { return (uint32_t)addReference(); }
-        SLANG_NO_THROW ULONG SLANG_MCALL Release() SLANG_OVERRIDE { return (uint32_t)releaseReference(); }
+        SLANG_NO_THROW ULONG SLANG_MCALL AddRef() SLANG_OVERRIDE { return 1; }
+        SLANG_NO_THROW ULONG SLANG_MCALL Release() SLANG_OVERRIDE { return 1; }
 
         // Implement IDxcIncludeHandler
         virtual HRESULT SLANG_MCALL LoadSource(LPCWSTR inFilename, IDxcBlob** outSource) SLANG_OVERRIDE
@@ -270,7 +269,7 @@ namespace Slang
 
         const String sourcePath = calcSourcePathForEntryPoint(endToEndReq, entryPointIndex);
 
-        ComPtr<DxcIncludeHandler> includeHandler(new DxcIncludeHandler(&linkage->searchDirectories, linkage->getFileSystemExt()));
+        DxcIncludeHandler includeHandler(&linkage->searchDirectories, linkage->getFileSystemExt(), compileRequest->getSourceManager());
 
         ComPtr<IDxcOperationResult> dxcResult;
         SLANG_RETURN_ON_FAIL(dxcCompiler->Compile(dxcSourceBlob,
@@ -279,9 +278,9 @@ namespace Slang
             wideProfileName.begin(),
             args,
             argCount,
-            nullptr,        // `#define`s
-            0,              // `#define` count
-            includeHandler, // `#include` handler
+            nullptr,            // `#define`s
+            0,                  // `#define` count
+            &includeHandler,    // `#include` handler
             dxcResult.writeRef()));
 
         // Retrieve result.

source/slang/slang-preprocessor.cpp

@@ -203,9 +203,8 @@ struct Preprocessor
     // diagnostics sink to use when writing messages
     DiagnosticSink*                         sink;
 
-    // An external callback interface to use when looking
-    // for files in a `#include` directive
-    IncludeHandler*                         includeHandler;
+    // Functionality for looking up files in a `#include` directive
+    IncludeSystem*                          includeSystem;
 
     // Current input stream (top of the stack of input)
     PreprocessorInputStream*                inputStream;
@@ -1818,8 +1817,8 @@ static void HandleIncludeDirective(PreprocessorDirectiveContext* context)
 
     PathInfo includedFromPathInfo = context->preprocessor->getSourceManager()->getPathInfo(directiveLoc, SourceLocType::Actual);
 
-    IncludeHandler* includeHandler = context->preprocessor->includeHandler;
-    if (!includeHandler)
+    IncludeSystem* includeSystem = context->preprocessor->includeSystem;
+    if (!includeSystem)
     {
         GetSink(context)->diagnose(pathToken.loc, Diagnostics::includeFailed, path);
         GetSink(context)->diagnose(pathToken.loc, Diagnostics::noIncludeHandlerSpecified);
@@ -1828,7 +1827,7 @@ static void HandleIncludeDirective(PreprocessorDirectiveContext* context)
 
     /* Find the path relative to the foundPath */
     PathInfo filePathInfo;
-    if (SLANG_FAILED(includeHandler->findFile(path, includedFromPathInfo.foundPath, filePathInfo)))
+    if (SLANG_FAILED(includeSystem->findFile(path, includedFromPathInfo.foundPath, filePathInfo)))
     {
         GetSink(context)->diagnose(pathToken.loc, Diagnostics::includeFailed, path);
         return;
@@ -1853,7 +1852,7 @@ static void HandleIncludeDirective(PreprocessorDirectiveContext* context)
     }
 
     // Simplify the path
-    filePathInfo.foundPath = includeHandler->simplifyPath(filePathInfo.foundPath);
+    filePathInfo.foundPath = includeSystem->simplifyPath(filePathInfo.foundPath);
 
     // Push the new file onto our stack of input streams
     // TODO(tfoley): check if we have made our include stack too deep
@@ -2355,7 +2354,7 @@ static void InitializePreprocessor(
     DiagnosticSink* sink)
 {
     preprocessor->sink = sink;
-    preprocessor->includeHandler = NULL;
+    preprocessor->includeSystem = NULL;
     preprocessor->endOfFileToken.type = TokenType::EndOfFile;
     preprocessor->endOfFileToken.flags = TokenFlag::AtStartOfLine;
 }
@@ -2451,7 +2450,7 @@ static TokenList ReadAllTokens(
 TokenList preprocessSource(
     SourceFile*                 file,
     DiagnosticSink*             sink,
-    IncludeHandler*             includeHandler,
+    IncludeSystem*              includeSystem,
     Dictionary<String, String>  defines,
     Linkage*                    linkage,
     Module*                     parentModule)
@@ -2461,7 +2460,7 @@ TokenList preprocessSource(
     preprocessor.linkage = linkage;
     preprocessor.parentModule = parentModule;
 
-    preprocessor.includeHandler = includeHandler;
+    preprocessor.includeSystem = includeSystem;
     for (auto p : defines)
     {
         DefineMacro(&preprocessor, p.Key, p.Value);

source/slang/slang-preprocessor.h

@@ -15,36 +15,11 @@ class Linkage;
 class Module;
 class ModuleDecl;
 
-// Callback interface for the preprocessor to use when looking
-// for files in `#include` directives.
-struct IncludeHandler
-{
-    virtual SlangResult findFile(const String& pathToInclude, const String& pathIncludedFrom, PathInfo& outPathInfo) = 0;
-    virtual String simplifyPath(const String& path) = 0;
-};
-
-// A default implementation that uses IncludeSystem to implement functionality
-struct IncludeHandlerImpl : IncludeHandler
-{
-    virtual SlangResult findFile(const String& pathToInclude, const String& pathIncludedFrom, PathInfo& outPathInfo) override
-    {
-        return m_system.findFile(pathToInclude, pathIncludedFrom, outPathInfo);
-    }
-    virtual String simplifyPath(const String& path) override { return m_system.simplifyPath(path); }
-
-    IncludeHandlerImpl(SearchDirectoryList* searchDirectories, ISlangFileSystemExt* fileSystemExt, SourceManager* sourceManager = nullptr) :
-        m_system(searchDirectories, fileSystemExt, sourceManager)
-    {
-    }
-protected:
-    IncludeSystem m_system;
-};
-
 // Take a string of source code and preprocess it into a list of tokens.
 TokenList preprocessSource(
     SourceFile*                 file,
     DiagnosticSink*             sink,
-    IncludeHandler*             includeHandler,
+    IncludeSystem*              includeSystem,
     Dictionary<String, String>  defines,
     Linkage*                    linkage,
     Module*                     parentModule);

source/slang/slang.cpp

@@ -897,8 +897,7 @@ void FrontEndCompileRequest::parseTranslationUnit(
     // Here we should probably be using the searchDirectories on the FrontEndCompileRequest.
     // If searchDirectories.parent pointed to the one in the Linkage would mean linkage paths
     // would be checked too (after those on the FrontEndCompileRequest). 
-
-    IncludeHandlerImpl includeHandler(&linkage->searchDirectories, linkage->getFileSystemExt());
+    IncludeSystem includeSystem(&linkage->searchDirectories, linkage->getFileSystemExt(), linkage->getSourceManager());
 
     RefPtr<Scope> languageScope;
     switch (translationUnit->sourceLanguage)
@@ -956,7 +955,7 @@ void FrontEndCompileRequest::parseTranslationUnit(
         auto tokens = preprocessSource(
             sourceFile,
             getSink(),
-            &includeHandler,
+            &includeSystem,
             combinedPreprocessorDefinitions,
             getLinkage(),
             module);
@@ -1651,14 +1650,14 @@ RefPtr<Module> Linkage::findOrImportModule(
     // Next, try to find the file of the given name,
     // using our ordinary include-handling logic.
 
-    IncludeHandlerImpl includeHandler(&searchDirectories, getFileSystemExt());
+    IncludeSystem includeSystem(&searchDirectories, getFileSystemExt(), getSourceManager());
 
     // Get the original path info
     PathInfo pathIncludedFromInfo = getSourceManager()->getPathInfo(loc, SourceLocType::Actual);
     PathInfo filePathInfo;
 
     // We have to load via the found path - as that is how file was originally loaded 
-    if (SLANG_FAILED(includeHandler.findFile(fileName, pathIncludedFromInfo.foundPath, filePathInfo)))
+    if (SLANG_FAILED(includeSystem.findFile(fileName, pathIncludedFromInfo.foundPath, filePathInfo)))
     {
         sink->diagnose(loc, Diagnostics::cannotFindFile, fileName);
         mapNameToLoadedModules[name] = nullptr;
@@ -1671,7 +1670,7 @@ RefPtr<Module> Linkage::findOrImportModule(
 
     // Try to load it
     ComPtr<ISlangBlob> fileContents;
-    if(SLANG_FAILED(getFileSystemExt()->loadFile(filePathInfo.foundPath.getBuffer(), fileContents.writeRef())))
+    if(SLANG_FAILED(includeSystem.loadFile(filePathInfo, fileContents)))
     {
         sink->diagnose(loc, Diagnostics::cannotOpenFile, fileName);
         mapNameToLoadedModules[name] = nullptr;

tools/slang-test/options.cpp

@@ -266,15 +266,15 @@ static bool _isSubCommand(const char* arg)
                 return res;
             }
         }
-        else if (strcmp(arg, "-nv-api-path") == 0)
+        else if (strcmp(arg, "-nvapi-path") == 0)
         {
             if (argCursor == argEnd)
             {
                 stdError.print("error: expected operand for '%s'\n", arg);
                 return SLANG_FAIL;
             }
 
-            optionsOut->nvAPIPath = *argCursor++;
+            optionsOut->nvapiPath = *argCursor++;
         }
         else
         {

tools/slang-test/options.h

@@ -98,7 +98,8 @@ struct Options
     // The adapter to use. If empty will match first found adapter.
     Slang::String adapter;
 
-    Slang::String nvAPIPath;
+    // The path to NVAPI if available. 
+    Slang::String nvapiPath;
 
         /// Parse the args, report any errors into stdError, and write the results into optionsOut
     static SlangResult parse(int argc, char** argv, TestCategorySet* categorySet, Slang::WriterHelper stdError, Options* optionsOut);

tools/slang-test/slang-test-main.cpp

@@ -3275,9 +3275,9 @@ SlangResult innerMain(int argc, char** argv)
         TestToolUtil::PreludeInfo info;
         info.exePath = argv[0];
 
-        if (options.nvAPIPath.getLength())
+        if (options.nvapiPath.getLength())
         {
-            info.nvAPIPath = options.nvAPIPath.getBuffer();
+            info.nvapiPath = options.nvapiPath.getBuffer();
         }
 
         TestToolUtil::setSessionDefaultPrelude(info, context.getSession());