From c2653ba1ec8f06453f0653d141bcd348eff5d4a5 Mon Sep 17 00:00:00 2001 From: jsmall-nvidia Date: Tue, 2 Mar 2021 17:03:16 -0500 Subject: Fix issue with long identifier names in GLSL output (#1731) * #include an absolute path didn't work - because paths were taken to always be relative. * First pass at handling 'names' that are too long in GLSL output. * Test to check functionality with very long func name. * Add access a long names buffer. * Fix typo in assert. Fix issue with coercion error for 1.0f / 0x7fffffff Co-authored-by: Tim Foley --- source/core/slang-string.cpp | 45 +++++++++++++++++++++++++++++++ source/core/slang-string.h | 4 ++- source/slang/slang-emit-c-like.cpp | 31 +++++++++++++++++++++ tests/bugs/token-limit.slang | 33 +++++++++++++++++++++++ tests/bugs/token-limit.slang.expected.txt | 4 +++ 5 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 tests/bugs/token-limit.slang create mode 100644 tests/bugs/token-limit.slang.expected.txt diff --git a/source/core/slang-string.cpp b/source/core/slang-string.cpp index a7374d8ba..62bc19754 100644 --- a/source/core/slang-string.cpp +++ b/source/core/slang-string.cpp @@ -340,6 +340,51 @@ namespace Slang } } + void String::reduceLength(Index newLength) + { + Index oldLength = getLength(); + SLANG_ASSERT(newLength <= oldLength); + if (oldLength == newLength) + { + return; + } + + // It must have a buffer, because only 0 length allows for nullptr + // and being 0 sized is already covered + SLANG_ASSERT(m_buffer); + + if (m_buffer->isUniquelyReferenced()) + { + m_buffer->length = newLength; + m_buffer->getData()[newLength] = 0; + } + else + { + // If 0 length is wanted we can just free + if (newLength == 0) + { + m_buffer.setNull(); + } + else + { + // We need to make a new copy, that we will shrink + + // We'll just go with capacity enough for the new length + const Index newCapacity = newLength; + StringRepresentation* newRepresentation = StringRepresentation::createWithCapacityAndLength(newCapacity, newLength); + + // Copy + char* dst = newRepresentation->getData(); + memcpy(dst, m_buffer->getData(), sizeof(char) * newLength); + // Zero terminate + dst[newLength] = 0; + + // Set the new rep + m_buffer = newRepresentation; + } + } + } + void String::append(const char* textBegin, char const* textEnd) { auto oldLength = getLength(); diff --git a/source/core/slang-string.h b/source/core/slang-string.h index 2312c0439..f42e78ac4 100644 --- a/source/core/slang-string.h +++ b/source/core/slang-string.h @@ -559,7 +559,9 @@ namespace Slang { return m_buffer ? m_buffer->getLength() : 0; } - + /// Make the length of the string the amount specified. Must be less than current size + void reduceLength(Index length); + friend String operator+(const char*op1, const String & op2); friend String operator+(const String & op1, const char * op2); friend String operator+(const String & op1, const String & op2); diff --git a/source/slang/slang-emit-c-like.cpp b/source/slang/slang-emit-c-like.cpp index d099af255..8e106fe55 100644 --- a/source/slang/slang-emit-c-like.cpp +++ b/source/slang/slang-emit-c-like.cpp @@ -620,6 +620,37 @@ void CLikeSourceEmitter::appendScrubbedName(const UnownedStringSlice& name, Stri out.appendChar(c); prevChar = c; } + + if (getSourceLanguage() == SourceLanguage::GLSL) + { + // It looks like the default glslang name limit is 1024, but let's go a little less so there is some wiggle room + const Index maxTokenLength = 1024 - 8; + + const Index length = out.getLength(); + + if (length > maxTokenLength) + { + // We are going to output with a prefix and a hash of the full name + const HashCode64 hash = getStableHashCode64(out.getBuffer(), length); + // Two hex chars per byte + const Index hashSize = sizeof(hash) * 2; + + // Work out a size that is within range taking into account the hash size and extra chars + Index reducedBaseLength = maxTokenLength - hashSize - 1; + // If it has a trailing _ remove it. + // We know because of scrubbing there can only be single _ + reducedBaseLength -= Index(out[reducedBaseLength - 1] == '_'); + + // Reduce the length + out.reduceLength(reducedBaseLength); + // Let's add a _ to separate from the rest of the name + out.appendChar('_'); + // Append the hash in hex + out.append(uint64_t(hash), 16); + + SLANG_ASSERT(out.getLength() <= maxTokenLength); + } + } } String CLikeSourceEmitter::generateEntryPointNameImpl(IREntryPointDecoration* entryPointDecor) diff --git a/tests/bugs/token-limit.slang b/tests/bugs/token-limit.slang new file mode 100644 index 000000000..bab18575c --- /dev/null +++ b/tests/bugs/token-limit.slang @@ -0,0 +1,33 @@ +//TEST(compute):COMPARE_COMPUTE: -shaderobj +//TEST(compute):COMPARE_COMPUTE:-cpu -shaderobj +//TEST(compute):COMPARE_COMPUTE:-vk -shaderobj + +// To test we want to generate names for things that are really long. Instead of repeated typing, we'll use macros here +// Build up a 2048 byte name +#define LONG_NAME abcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGH + +#define CONCAT(A, B) A ## B + +//TEST_INPUT:ubuffer(data=[1 2 3 4], stride=4):name=inputBuffer +RWStructuredBuffer inputBuffer; + +//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name=outputBuffer +RWStructuredBuffer outputBuffer; + +#define workBuffer CONCAT(LONG_NAME, WorkBuffer) +#define addFunc CONCAT(LONG_NAME, Add) + +//TEST_INPUT:ubuffer(data=[0 7 8 2], stride=4):name=abcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHabcdefghABCDEFGHWorkBuffer +RWStructuredBuffer workBuffer; + +int addFunc(int a, int b) { return a + b; } + +[numthreads(4, 1, 1)] +void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID) +{ + uint tid = dispatchThreadID.x; + + int value = inputBuffer[tid]; + + outputBuffer[tid] = addFunc(workBuffer[tid], value); +} diff --git a/tests/bugs/token-limit.slang.expected.txt b/tests/bugs/token-limit.slang.expected.txt new file mode 100644 index 000000000..10219cada --- /dev/null +++ b/tests/bugs/token-limit.slang.expected.txt @@ -0,0 +1,4 @@ +1 +9 +B +6 -- cgit v1.2.3