Skip to content

Commit d8daeb4

Browse files
johnkslangjohnkslang
committed
Non-functional: correctly do GL_EXT_buffer_reference2 semantic checking
See KhronosGroup#2366 for detail.
1 parent 6c37bbb commit d8daeb4

File tree

8 files changed

+1245
-1232
lines changed

8 files changed

+1245
-1232
lines changed

‎Test/baseResults/spv.bufferhandle17_Errors.frag.out

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,12 @@ spv.bufferhandle17_Errors.frag
22
ERROR: 0:11: 'qualifier' : variables with reference type can't have qualifier 'const'
33
ERROR: 0:16: 'qualifier' : variables with reference type can't have qualifier 'const'
44
ERROR: 0:18: '==' : can't use with reference types
5+
ERROR: 0:18: 'buffer reference math' : required extension not requested: GL_EXT_buffer_reference2
6+
ERROR: 0:18: '==' : wrong operand types: no operation '==' exists that takes a left-hand operand of type ' temp reference' and a right operand of type ' temp reference' (or there is no acceptable conversion)
57
ERROR: 0:19: '!=' : can't use with reference types
6-
ERROR: 4 compilation errors. No code generated.
8+
ERROR: 0:19: 'buffer reference math' : required extension not requested: GL_EXT_buffer_reference2
9+
ERROR: 0:19: '!=' : wrong operand types: no operation '!=' exists that takes a left-hand operand of type ' temp reference' and a right operand of type ' temp reference' (or there is no acceptable conversion)
10+
ERROR: 8 compilation errors. No code generated.
711

812

913
SPIR-V is not generated for failed compile or link

‎glslang/MachineIndependent/Intermediate.cpp

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ TIntermTyped* TIntermediate::addBinaryMath(TOperator op, TIntermTyped* left, TIn
120120
return nullptr;
121121

122122
// Convert "reference +/- int" and "reference - reference" to integer math
123-
if ((op == EOpAdd || op == EOpSub) && extensionRequested(E_GL_EXT_buffer_reference2)) {
123+
if (op == EOpAdd || op == EOpSub) {
124124

125125
// No addressing math on struct with unsized array.
126126
if ((left->isReference() && left->getType().getReferentType()->containsUnsizedArray()) ||
@@ -140,41 +140,42 @@ TIntermTyped* TIntermediate::addBinaryMath(TOperator op, TIntermTyped* left, TIn
140140
node = addBuiltInFunctionCall(loc, EOpConvUint64ToPtr, true, node, referenceType);
141141
return node;
142142
}
143+
}
143144

144-
if (op == EOpAdd && right->isReference() && isTypeInt(left->getBasicType())) {
145-
const TType& referenceType = right->getType();
146-
TIntermConstantUnion* size = addConstantUnion((unsigned long long)computeBufferReferenceTypeSize(right->getType()), loc, true);
147-
right = addBuiltInFunctionCall(loc, EOpConvPtrToUint64, true, right, TType(EbtUint64));
148-
149-
left = createConversion(EbtInt64, left);
150-
left = addBinaryMath(EOpMul, left, size, loc);
145+
if (op == EOpAdd && right->isReference() && isTypeInt(left->getBasicType())) {
146+
const TType& referenceType = right->getType();
147+
TIntermConstantUnion* size =
148+
addConstantUnion((unsigned long long)computeBufferReferenceTypeSize(right->getType()), loc, true);
149+
right = addBuiltInFunctionCall(loc, EOpConvPtrToUint64, true, right, TType(EbtUint64));
151150

152-
TIntermTyped *node = addBinaryMath(op, left, right, loc);
153-
node = addBuiltInFunctionCall(loc, EOpConvUint64ToPtr, true, node, referenceType);
154-
return node;
155-
}
151+
left = createConversion(EbtInt64, left);
152+
left = addBinaryMath(EOpMul, left, size, loc);
156153

157-
if (op == EOpSub && left->isReference() && right->isReference()) {
158-
TIntermConstantUnion* size = addConstantUnion((long long)computeBufferReferenceTypeSize(left->getType()), loc, true);
154+
TIntermTyped *node = addBinaryMath(op, left, right, loc);
155+
node = addBuiltInFunctionCall(loc, EOpConvUint64ToPtr, true, node, referenceType);
156+
return node;
157+
}
159158

160-
left = addBuiltInFunctionCall(loc, EOpConvPtrToUint64, true, left, TType(EbtUint64));
161-
right = addBuiltInFunctionCall(loc, EOpConvPtrToUint64, true, right, TType(EbtUint64));
159+
if (op == EOpSub && left->isReference() && right->isReference()) {
160+
TIntermConstantUnion* size =
161+
addConstantUnion((long long)computeBufferReferenceTypeSize(left->getType()), loc, true);
162162

163-
left = addBuiltInFunctionCall(loc, EOpConvUint64ToInt64, true, left, TType(EbtInt64));
164-
right = addBuiltInFunctionCall(loc, EOpConvUint64ToInt64, true, right, TType(EbtInt64));
163+
left = addBuiltInFunctionCall(loc, EOpConvPtrToUint64, true, left, TType(EbtUint64));
164+
right = addBuiltInFunctionCall(loc, EOpConvPtrToUint64, true, right, TType(EbtUint64));
165165

166-
left = addBinaryMath(EOpSub, left, right, loc);
166+
left = addBuiltInFunctionCall(loc, EOpConvUint64ToInt64, true, left, TType(EbtInt64));
167+
right = addBuiltInFunctionCall(loc, EOpConvUint64ToInt64, true, right, TType(EbtInt64));
167168

168-
TIntermTyped *node = addBinaryMath(EOpDiv, left, size, loc);
169-
return node;
170-
}
169+
left = addBinaryMath(EOpSub, left, right, loc);
171170

172-
// No other math operators supported on references
173-
if (left->isReference() || right->isReference()) {
174-
return nullptr;
175-
}
171+
TIntermTyped *node = addBinaryMath(EOpDiv, left, size, loc);
172+
return node;
176173
}
177174

175+
// No other math operators supported on references
176+
if (left->isReference() || right->isReference())
177+
return nullptr;
178+
178179
// Try converting the children's base types to compatible types.
179180
auto children = addConversion(op, left, right);
180181
left = std::get<0>(children);
@@ -290,9 +291,7 @@ TIntermTyped* TIntermediate::addAssign(TOperator op, TIntermTyped* left, TInterm
290291
// Convert "reference += int" to "reference = reference + int". We need this because the
291292
// "reference + int" calculation involves a cast back to the original type, which makes it
292293
// not an lvalue.
293-
if ((op == EOpAddAssign || op == EOpSubAssign) && left->isReference() &&
294-
extensionRequested(E_GL_EXT_buffer_reference2)) {
295-
294+
if ((op == EOpAddAssign || op == EOpSubAssign) && left->isReference()) {
296295
if (!(right->getType().isScalar() && right->getType().isIntegerDomain()))
297296
return nullptr;
298297

‎glslang/MachineIndependent/ParseHelper.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,8 +751,11 @@ TIntermTyped* TParseContext::handleBinaryMath(const TSourceLoc& loc, const char*
751751
}
752752

753753
TIntermTyped* result = nullptr;
754-
if (allowed)
754+
if (allowed) {
755+
if ((left->isReference() || right->isReference()))
756+
requireExtensions(loc, 1, &E_GL_EXT_buffer_reference2, "buffer reference math");
755757
result = intermediate.addBinaryMath(op, left, right, loc);
758+
}
756759

757760
if (result == nullptr)
758761
binaryOpError(loc, str, left->getCompleteString(), right->getCompleteString());
@@ -1680,6 +1683,14 @@ TIntermTyped* TParseContext::addOutputArgumentConversions(const TFunction& funct
16801683
#endif
16811684
}
16821685

1686+
TIntermTyped* TParseContext::addAssign(const TSourceLoc& loc, TOperator op, TIntermTyped* left, TIntermTyped* right)
1687+
{
1688+
if ((op == EOpAddAssign || op == EOpSubAssign) && left->isReference())
1689+
requireExtensions(loc, 1, &E_GL_EXT_buffer_reference2, "+= and -= on a buffer reference");
1690+
1691+
return intermediate.addAssign(op, left, right, loc);
1692+
}
1693+
16831694
void TParseContext::memorySemanticsCheck(const TSourceLoc& loc, const TFunction& fnCandidate, const TIntermOperator& callNode)
16841695
{
16851696
const TIntermSequence* argp = &callNode.getAsAggregate()->getSequence();

‎glslang/MachineIndependent/ParseHelper.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,7 @@ class TParseContext : public TParseContextBase {
328328
TIntermTyped* handleLengthMethod(const TSourceLoc&, TFunction*, TIntermNode*);
329329
void addInputArgumentConversions(const TFunction&, TIntermNode*&) const;
330330
TIntermTyped* addOutputArgumentConversions(const TFunction&, TIntermAggregate&) const;
331+
TIntermTyped* addAssign(const TSourceLoc&, TOperator op, TIntermTyped* left, TIntermTyped* right);
331332
void builtInOpCheck(const TSourceLoc&, const TFunction&, TIntermOperator&);
332333
void nonOpBuiltInCheck(const TSourceLoc&, const TFunction&, TIntermAggregate&);
333334
void userFunctionCallCheck(const TSourceLoc&, TIntermAggregate&);

‎glslang/MachineIndependent/glslang.m4

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -778,7 +778,7 @@ assignment_expression
778778
parseContext.specializationCheck($2.loc, $1->getType(), "=");
779779
parseContext.lValueErrorCheck($2.loc, "assign", $1);
780780
parseContext.rValueErrorCheck($2.loc, "assign", $3);
781-
$$ = parseContext.intermediate.addAssign($2.op, $1, $3, $2.loc);
781+
$$ = parseContext.addAssign($2.loc, $2.op, $1, $3);
782782
if ($$ == 0) {
783783
parseContext.assignError($2.loc, "assign", $1->getCompleteString(), $3->getCompleteString());
784784
$$ = $1;

‎glslang/MachineIndependent/glslang.y

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -778,7 +778,7 @@ assignment_expression
778778
parseContext.specializationCheck($2.loc, $1->getType(), "=");
779779
parseContext.lValueErrorCheck($2.loc, "assign", $1);
780780
parseContext.rValueErrorCheck($2.loc, "assign", $3);
781-
$$ = parseContext.intermediate.addAssign($2.op, $1, $3, $2.loc);
781+
$$ = parseContext.addAssign($2.loc, $2.op, $1, $3);
782782
if ($$ == 0) {
783783
parseContext.assignError($2.loc, "assign", $1->getCompleteString(), $3->getCompleteString());
784784
$$ = $1;
@@ -3885,4 +3885,3 @@ single_attribute
38853885

38863886

38873887
%%
3888-

0 commit comments

Comments
 (0)