From 75c5603ada785ba9e9826c8c1023c55c775b2ec9 Mon Sep 17 00:00:00 2001 From: Rex Xu Date: Mon, 14 Jan 2019 11:40:51 +0800 Subject: [PATCH] Fix xfb_stride incorrectness(#1654) Add int64 support in XFB. Change containsDouble to contains64BitType. Make it more general. --- Test/baseResults/440.vert.out | 2 +- glslang/MachineIndependent/ParseHelper.cpp | 8 ++--- glslang/MachineIndependent/linkValidate.cpp | 44 +++++++++++++------------- glslang/MachineIndependent/localintermediate.h | 6 ++-- hlsl/hlslParseHelper.cpp | 8 ++--- 5 files changed, 34 insertions(+), 34 deletions(-) diff --git a/Test/baseResults/440.vert.out b/Test/baseResults/440.vert.out index 5a10e26..09c6af3 100644 --- a/Test/baseResults/440.vert.out +++ b/Test/baseResults/440.vert.out @@ -166,7 +166,7 @@ Linked vertex stage: ERROR: Linking vertex stage: Missing entry point: Each stage requires one entry point ERROR: Linking vertex stage: xfb_stride is too small to hold all buffer entries: ERROR: xfb_buffer 0, xfb_stride 92, minimum stride needed: 96 -ERROR: Linking vertex stage: xfb_stride must be multiple of 8 for buffer holding a double: +ERROR: Linking vertex stage: xfb_stride must be multiple of 8 for buffer holding a double or 64-bit integer: ERROR: xfb_buffer 0, xfb_stride 92 ERROR: Linking vertex stage: xfb_stride must be multiple of 4: ERROR: xfb_buffer 5, xfb_stride 6 diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index d16dc99..62d6694 100755 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/glslang/MachineIndependent/ParseHelper.cpp @@ -7270,12 +7270,12 @@ void TParseContext::fixXfbOffsets(TQualifier& qualifier, TTypeList& typeList) int nextOffset = qualifier.layoutXfbOffset; for (unsigned int member = 0; member < typeList.size(); ++member) { TQualifier& memberQualifier = typeList[member].type->getQualifier(); - bool containsDouble = false; - int memberSize = intermediate.computeTypeXfbSize(*typeList[member].type, containsDouble); + bool contains64BitType = false; + int memberSize = intermediate.computeTypeXfbSize(*typeList[member].type, contains64BitType); // see if we need to auto-assign an offset to this member if (! memberQualifier.hasXfbOffset()) { - // "if applied to an aggregate containing a double, the offset must also be a multiple of 8" - if (containsDouble) + // "if applied to an aggregate containing a double or 64-bit integer, the offset must also be a multiple of 8" + if (contains64BitType) RoundToPow2(nextOffset, 8); memberQualifier.layoutXfbOffset = nextOffset; } else diff --git a/glslang/MachineIndependent/linkValidate.cpp b/glslang/MachineIndependent/linkValidate.cpp index 977600b..b7d8545 100755 --- a/glslang/MachineIndependent/linkValidate.cpp +++ b/glslang/MachineIndependent/linkValidate.cpp @@ -222,8 +222,8 @@ void TIntermediate::mergeModes(TInfoSink& infoSink, TIntermediate& unit) else if (xfbBuffers[b].stride != unit.xfbBuffers[b].stride) error(infoSink, "Contradictory xfb_stride"); xfbBuffers[b].implicitStride = std::max(xfbBuffers[b].implicitStride, unit.xfbBuffers[b].implicitStride); - if (unit.xfbBuffers[b].containsDouble) - xfbBuffers[b].containsDouble = true; + if (unit.xfbBuffers[b].contains64BitType) + xfbBuffers[b].contains64BitType = true; // TODO: 4.4 link: enhanced layouts: compare ranges } @@ -634,7 +634,7 @@ void TIntermediate::finalCheck(TInfoSink& infoSink, bool keepUncalled) error(infoSink, "Cannot use both gl_FragColor and gl_FragData"); for (size_t b = 0; b < xfbBuffers.size(); ++b) { - if (xfbBuffers[b].containsDouble) + if (xfbBuffers[b].contains64BitType) RoundToPow2(xfbBuffers[b].implicitStride, 8); // "It is a compile-time or link-time error to have @@ -650,10 +650,10 @@ void TIntermediate::finalCheck(TInfoSink& infoSink, bool keepUncalled) xfbBuffers[b].stride = xfbBuffers[b].implicitStride; // "If the buffer is capturing any - // outputs with double-precision components, the stride must be a multiple of 8, otherwise it must be a + // outputs with double-precision or 64-bit integer components, the stride must be a multiple of 8, otherwise it must be a // multiple of 4, or a compile-time or link-time error results." - if (xfbBuffers[b].containsDouble && ! IsMultipleOfPow2(xfbBuffers[b].stride, 8)) { - error(infoSink, "xfb_stride must be multiple of 8 for buffer holding a double:"); + if (xfbBuffers[b].contains64BitType && ! IsMultipleOfPow2(xfbBuffers[b].stride, 8)) { + error(infoSink, "xfb_stride must be multiple of 8 for buffer holding a double or 64-bit integer:"); infoSink.info.prefix(EPrefixError); infoSink.info << " xfb_buffer " << (unsigned int)b << ", xfb_stride " << xfbBuffers[b].stride << "\n"; } else if (! IsMultipleOfPow2(xfbBuffers[b].stride, 4)) { @@ -1260,7 +1260,7 @@ int TIntermediate::addXfbBufferOffset(const TType& type) TXfbBuffer& buffer = xfbBuffers[qualifier.layoutXfbBuffer]; // compute the range - unsigned int size = computeTypeXfbSize(type, buffer.containsDouble); + unsigned int size = computeTypeXfbSize(type, buffer.contains64BitType); buffer.implicitStride = std::max(buffer.implicitStride, qualifier.layoutXfbOffset + size); TRange range(qualifier.layoutXfbOffset, qualifier.layoutXfbOffset + size - 1); @@ -1279,11 +1279,11 @@ int TIntermediate::addXfbBufferOffset(const TType& type) // Recursively figure out how many bytes of xfb buffer are used by the given type. // Return the size of type, in bytes. -// Sets containsDouble to true if the type contains a double. -// N.B. Caller must set containsDouble to false before calling. -unsigned int TIntermediate::computeTypeXfbSize(const TType& type, bool& containsDouble) const +// Sets contains64BitType to true if the type contains a double. +// N.B. Caller must set contains64BitType to false before calling. +unsigned int TIntermediate::computeTypeXfbSize(const TType& type, bool& contains64BitType) const { - // "...if applied to an aggregate containing a double, the offset must also be a multiple of 8, + // "...if applied to an aggregate containing a double or 64-bit integer, the offset must also be a multiple of 8, // and the space taken in the buffer will be a multiple of 8. // ...within the qualified entity, subsequent components are each // assigned, in order, to the next available offset aligned to a multiple of @@ -1294,28 +1294,28 @@ unsigned int TIntermediate::computeTypeXfbSize(const TType& type, bool& contains // TODO: perf: this can be flattened by using getCumulativeArraySize(), and a deref that discards all arrayness assert(type.isSizedArray()); TType elementType(type, 0); - return type.getOuterArraySize() * computeTypeXfbSize(elementType, containsDouble); + return type.getOuterArraySize() * computeTypeXfbSize(elementType, contains64BitType); } if (type.isStruct()) { unsigned int size = 0; - bool structContainsDouble = false; + bool structContains64BitType = false; for (int member = 0; member < (int)type.getStruct()->size(); ++member) { TType memberType(type, member); // "... if applied to - // an aggregate containing a double, the offset must also be a multiple of 8, + // an aggregate containing a double or 64-bit integer, the offset must also be a multiple of 8, // and the space taken in the buffer will be a multiple of 8." - bool memberContainsDouble = false; - int memberSize = computeTypeXfbSize(memberType, memberContainsDouble); - if (memberContainsDouble) { - structContainsDouble = true; + bool memberContains64BitType = false; + int memberSize = computeTypeXfbSize(memberType, memberContains64BitType); + if (memberContains64BitType) { + structContains64BitType = true; RoundToPow2(size, 8); } size += memberSize; } - if (structContainsDouble) { - containsDouble = true; + if (structContains64BitType) { + contains64BitType = true; RoundToPow2(size, 8); } return size; @@ -1333,8 +1333,8 @@ unsigned int TIntermediate::computeTypeXfbSize(const TType& type, bool& contains numComponents = 1; } - if (type.getBasicType() == EbtDouble) { - containsDouble = true; + if (type.getBasicType() == EbtDouble || type.getBasicType() == EbtInt64 || type.getBasicType() == EbtUint64) { + contains64BitType = true; return 8 * numComponents; } else return 4 * numComponents; diff --git a/glslang/MachineIndependent/localintermediate.h b/glslang/MachineIndependent/localintermediate.h index c114f21..4147c60 100755 --- a/glslang/MachineIndependent/localintermediate.h +++ b/glslang/MachineIndependent/localintermediate.h @@ -149,11 +149,11 @@ struct TOffsetRange { // Things that need to be tracked per xfb buffer. struct TXfbBuffer { - TXfbBuffer() : stride(TQualifier::layoutXfbStrideEnd), implicitStride(0), containsDouble(false) { } + TXfbBuffer() : stride(TQualifier::layoutXfbStrideEnd), implicitStride(0), contains64BitType(false) { } std::vector ranges; // byte offsets that have already been assigned unsigned int stride; unsigned int implicitStride; - bool containsDouble; + bool contains64BitType; }; // Track a set of strings describing how the module was processed. @@ -640,7 +640,7 @@ public: } unsigned getXfbStride(int buffer) const { return xfbBuffers[buffer].stride; } int addXfbBufferOffset(const TType&); - unsigned int computeTypeXfbSize(const TType&, bool& containsDouble) const; + unsigned int computeTypeXfbSize(const TType&, bool& contains64BitType) const; static int getBaseAlignmentScalar(const TType&, int& size); static int getBaseAlignment(const TType&, int& size, int& stride, TLayoutPacking layoutPacking, bool rowMajor); static int getScalarAlignment(const TType&, int& size, int& stride, bool rowMajor); diff --git a/hlsl/hlslParseHelper.cpp b/hlsl/hlslParseHelper.cpp index d6dc2e8..35f8721 100755 --- a/hlsl/hlslParseHelper.cpp +++ b/hlsl/hlslParseHelper.cpp @@ -8697,12 +8697,12 @@ void HlslParseContext::fixXfbOffsets(TQualifier& qualifier, TTypeList& typeList) int nextOffset = qualifier.layoutXfbOffset; for (unsigned int member = 0; member < typeList.size(); ++member) { TQualifier& memberQualifier = typeList[member].type->getQualifier(); - bool containsDouble = false; - int memberSize = intermediate.computeTypeXfbSize(*typeList[member].type, containsDouble); + bool contains64BitType = false; + int memberSize = intermediate.computeTypeXfbSize(*typeList[member].type, contains64BitType); // see if we need to auto-assign an offset to this member if (! memberQualifier.hasXfbOffset()) { - // "if applied to an aggregate containing a double, the offset must also be a multiple of 8" - if (containsDouble) + // "if applied to an aggregate containing a double or 64-bit integer, the offset must also be a multiple of 8" + if (contains64BitType) RoundToPow2(nextOffset, 8); memberQualifier.layoutXfbOffset = nextOffset; } else -- 2.7.4