From 3e312b3062ab493c80d7d7eddf43c94ec59ecdb7 Mon Sep 17 00:00:00 2001 From: jsmall-nvidia Date: Thu, 10 Nov 2022 18:38:19 -0500 Subject: Improvements to NVRTC diagnostic parsing (#2504) * #include an absolute path didn't work - because paths were taken to always be relative. * Float16 support for C++/CPU based targets with f16tof32 and f32tof16. * Small correction around INF/NAN handling for f32tof16 * Small improvement to f16tof32 * Disable CUDA test for now. * Improvements to NVRTC diagnostic parsing. Handle compilerSpecificArgs. Fix issue with terminating nul ending up in diagnostic string. * Improved NVRTC error parsing. f32tof16 and f16tof32 work in principal on CUDA. * Small update to test, although they remain disabled. * Work around SLANG_E_NOT_AVAILABLE being turned into ignored, when a legitimate error is found * A more tightly constrained fallback NVRTC diagnostic parsing. * Remove CharUtil, as not neeed. Co-authored-by: Yong He --- source/compiler-core/slang-nvrtc-compiler.cpp | 136 +++++++++++++++++++++++--- source/slang/hlsl.meta.slang | 4 + tests/hlsl-intrinsic/f16tof32.slang | 2 +- tests/hlsl-intrinsic/f32tof16.slang | 2 +- tools/render-test/render-test-main.cpp | 11 ++- 5 files changed, 139 insertions(+), 16 deletions(-) diff --git a/source/compiler-core/slang-nvrtc-compiler.cpp b/source/compiler-core/slang-nvrtc-compiler.cpp index 1a112d67f..f650a11f5 100644 --- a/source/compiler-core/slang-nvrtc-compiler.cpp +++ b/source/compiler-core/slang-nvrtc-compiler.cpp @@ -14,6 +14,7 @@ #include "../core/slang-semantic-version.h" #include "../core/slang-shared-library.h" +#include "../core/slang-char-util.h" #include "slang-artifact-diagnostic-util.h" #include "slang-artifact-util.h" @@ -228,24 +229,79 @@ static SlangResult _parseNVRTCLine(SliceAllocator& allocator, const UnownedStrin if (split.getCount() >= 3) { // tests/cuda/cuda-compile.cu(7): warning: variable "c" is used before its value is set - const auto split1 = split[1].trim(); - if (split1 == "error") + Severity severity = Severity::Unknown; + + if (split1 == toSlice("error") || + split1 == toSlice("catastrophic error")) { - outDiagnostic.severity = Severity::Error; + severity = Severity::Error; } - else if (split1 == "warning") + else if (split1 == toSlice("warning")) { - outDiagnostic.severity = Severity::Warning; + severity = Severity::Warning; } - outDiagnostic.text = allocator.allocate(split[2].trim()); + else + { + // Fall back position to try and determine if this really is some kind of + // error/warning without succeeding when it's due to some other property + // of the output diagnostics. + // + // Anything ending with " warning:" or " error:" in effect. + + // We can expand to include character after as this is split1, as must be followed by at a minimum + // : (as the split has at least 3 parts). + const UnownedStringSlice expandSplit1(split1.begin(), split1.end() + 1); - SLANG_RETURN_ON_FAIL(_parseLocation(allocator, split[0], outDiagnostic)); - return SLANG_OK; + if (expandSplit1.endsWith(toSlice(" error:"))) + { + severity = Severity::Error; + } + else if (expandSplit1.endsWith(toSlice(" warning:"))) + { + severity = Severity::Warning; + } + } + + if (severity != Severity::Unknown) + { + // The text is everything following the : after the warning. + UnownedStringSlice text(split[2].begin(), split.getLast().end()); + + // Trim whitespace at start and end + text = text.trim(); + + // Set the diagnostic + outDiagnostic.severity = severity; + outDiagnostic.text = allocator.allocate(text); + SLANG_RETURN_ON_FAIL(_parseLocation(allocator, split[0], outDiagnostic)); + + return SLANG_OK; + } + + // TODO(JS): Note here if it's not possible to determine a line as being the main diagnostics + // we fall through to it potentially being a note. + // + // That could mean a valid diagnostic (from NVRTCs point of view) is ignored/noted, because this code + // can't parse it. Ideally that situation would lead to an error such that we can detect + // and things will fail. + // + // So we might want to revisit this determination in the future. } - - return SLANG_E_NOT_FOUND; + + // There isn't a diagnostic on this line + if (line.getLength() == 0 || + line.trim().getLength() == 0) + { + return SLANG_E_NOT_FOUND; + } + + // We'll assume it's info, associated with a previous line + outDiagnostic.severity = Severity::Info; + outDiagnostic.text = allocator.allocate(line); + + return SLANG_OK; } /* An implementation of Path::Visitor that can be used for finding NVRTC shared library installations. */ @@ -819,6 +875,18 @@ SlangResult NVRTCDownstreamCompiler::compile(const DownstreamCompileOptions& opt cmdLine.addArg("-DSLANG_CUDA_ENABLE_OPTIX"); } + // Add any compiler specific options + // NOTE! If these clash with any previously set options (as set via other flags) + // compilation might fail. + if (options.compilerSpecificArguments.count > 0) + { + for (auto compilerSpecificArg : options.compilerSpecificArguments) + { + const char*const arg = compilerSpecificArg; + cmdLine.addArg(arg); + } + } + SLANG_ASSERT(headers.getCount() == headerIncludeNames.getCount()); ComPtr sourceBlob; @@ -868,6 +936,11 @@ SlangResult NVRTCDownstreamCompiler::compile(const DownstreamCompileOptions& opt { char* dst = rawDiagnostics.prepareForAppend(Index(logSize)); SLANG_NVRTC_RETURN_ON_FAIL(m_nvrtcGetProgramLog(program, dst)); + + // If there is a terminating zero remove it, as the rawDiagnostics + // string will already contain one. + logSize -= size_t(logSize > 0 && dst[logSize - 1] == 0); + rawDiagnostics.appendInPlace(dst, Index(logSize)); diagnostics->setRaw(SliceUtil::asCharSlice(rawDiagnostics)); @@ -875,24 +948,61 @@ SlangResult NVRTCDownstreamCompiler::compile(const DownstreamCompileOptions& opt SliceAllocator allocator; + // Get all of the lines + List lines; + StringUtil::calcLines(rawDiagnostics.getUnownedSlice(), lines); + + // Remove any trailing empty lines + while (lines.getCount() && lines.getLast().getLength() == 0) + { + lines.removeLast(); + } + + // Find the index searching from last line, that is blank + // indicating the end of the output + Index lastIndex = lines.getCount(); + + // Look for the first blank line after this point. + // We'll assume any information after that blank line to the end of the diagnostic + // is compilation summary information. + for (Index i = lastIndex - 1; i >= 0; --i) + { + if (lines[i].getLength() == 0) + { + lastIndex = i; + break; + } + } + // Parse the diagnostics here - for (auto line : LineParser(rawDiagnostics.getUnownedSlice())) + for (auto line : makeConstArrayView(lines.getBuffer(), lastIndex)) { ArtifactDiagnostic diagnostic; SlangResult lineRes = _parseNVRTCLine(allocator, line, diagnostic); if (SLANG_SUCCEEDED(lineRes)) { + // We only allow info diagnostics after a 'regular' diagnostic. + if (diagnostic.severity == ArtifactDiagnostic::Severity::Info && + diagnostics->getCount() == 0) + { + continue; + } + diagnostics->add(diagnostic); } else if (lineRes != SLANG_E_NOT_FOUND) { + // If there is an error exit + // But if SLANG_E_NOT_FOUND that just means this line couldn't be parsed, so ignore. return lineRes; } } - // if it has a compilation error.. set on output - if (diagnostics->hasOfAtLeastSeverity(ArtifactDiagnostic::Severity::Error)) + // If it has a compilation error.. and there isn't already an error set + // set as failed. + if (SLANG_SUCCEEDED(diagnostics->getResult()) && + diagnostics->hasOfAtLeastSeverity(ArtifactDiagnostic::Severity::Error)) { diagnostics->setResult(SLANG_FAIL); } diff --git a/source/slang/hlsl.meta.slang b/source/slang/hlsl.meta.slang index ec2e6de95..1cff7d6f3 100644 --- a/source/slang/hlsl.meta.slang +++ b/source/slang/hlsl.meta.slang @@ -1742,6 +1742,8 @@ matrix exp2(matrix x) __target_intrinsic(glsl, "unpackHalf2x16($0).x") __glsl_version(420) __target_intrinsic(hlsl) +__cuda_sm_version(6.0) +__target_intrinsic(cuda, "__half2float(__short_as_half($0))") float f16tof32(uint value); __generic @@ -1757,6 +1759,8 @@ vector f16tof32(vector value) __target_intrinsic(glsl, "packHalf2x16(vec2($0,0.0))") __glsl_version(420) __target_intrinsic(hlsl) +__cuda_sm_version(6.0) +__target_intrinsic(cuda, "__half_as_ushort(__float2half($0))") uint f32tof16(float value); __generic diff --git a/tests/hlsl-intrinsic/f16tof32.slang b/tests/hlsl-intrinsic/f16tof32.slang index b73ade4cf..78c5fdae6 100644 --- a/tests/hlsl-intrinsic/f16tof32.slang +++ b/tests/hlsl-intrinsic/f16tof32.slang @@ -2,7 +2,7 @@ //TEST(compute):COMPARE_COMPUTE_EX:-slang -compute //TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -dx12 //TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute -//DISABLE_TEST(compute):COMPARE_COMPUTE_EX:-cuda -compute +//DISABLE_TEST(compute):COMPARE_COMPUTE_EX:-cuda -compute -render-features half //TEST_INPUT:ubuffer(data=[0 0 0 0 0 0 0 0], stride=4):out,name outputBuffer RWStructuredBuffer outputBuffer; diff --git a/tests/hlsl-intrinsic/f32tof16.slang b/tests/hlsl-intrinsic/f32tof16.slang index 465b2840a..ebcb6b40a 100644 --- a/tests/hlsl-intrinsic/f32tof16.slang +++ b/tests/hlsl-intrinsic/f32tof16.slang @@ -2,7 +2,7 @@ //TEST(compute):COMPARE_COMPUTE_EX:-slang -compute //TEST(compute):COMPARE_COMPUTE_EX:-slang -compute -dx12 //TEST(compute, vulkan):COMPARE_COMPUTE_EX:-vk -compute -//DISABLE_TEST(compute):COMPARE_COMPUTE_EX:-cuda -compute +//DISABLE_TEST(compute):COMPARE_COMPUTE_EX:-cuda -compute -render-features half //TEST_INPUT:ubuffer(data=[0 0 0 0 0 0 0 0], stride=4):out,name outputBuffer RWStructuredBuffer outputBuffer; diff --git a/tools/render-test/render-test-main.cpp b/tools/render-test/render-test-main.cpp index 805f08d10..d3c284132 100644 --- a/tools/render-test/render-test-main.cpp +++ b/tools/render-test/render-test-main.cpp @@ -510,7 +510,16 @@ SlangResult RenderTestApp::initialize( // ComPtr outDiagnostics; auto result = device->createProgram(m_compilationOutput.output.desc, m_shaderProgram.writeRef(), outDiagnostics.writeRef()); - SLANG_RETURN_ON_FAIL(result); + + // If there was a failure creating a program, we can't continue + // Special case SLANG_E_NOT_AVAILABLE error code to make it a failure, + // as it is also used to indicate an attempt setup something failed gracefully (because it couldn't be supported) + // but that's not this. + if (SLANG_FAILED(result)) + { + result = (result == SLANG_E_NOT_AVAILABLE) ? SLANG_FAIL : result; + return result; + } m_device = device; -- cgit v1.2.3