Unverified Commit d2e9829a by John Kessenich Committed by GitHub

Merge pull request #1370 from KhronosGroup/fix-param-types

Fix #944: Convert argument type to match formal parameter type.
parents 115c3b14 d3ed90be
...@@ -170,7 +170,7 @@ protected: ...@@ -170,7 +170,7 @@ protected:
void declareUseOfStructMember(const glslang::TTypeList& members, int glslangMember); void declareUseOfStructMember(const glslang::TTypeList& members, int glslangMember);
bool isShaderEntryPoint(const glslang::TIntermAggregate* node); bool isShaderEntryPoint(const glslang::TIntermAggregate* node);
bool writableParam(glslang::TStorageQualifier); bool writableParam(glslang::TStorageQualifier) const;
bool originalParam(glslang::TStorageQualifier, const glslang::TType&, bool implicitThisParam); bool originalParam(glslang::TStorageQualifier, const glslang::TType&, bool implicitThisParam);
void makeFunctions(const glslang::TIntermSequence&); void makeFunctions(const glslang::TIntermSequence&);
void makeGlobalInitializers(const glslang::TIntermSequence&); void makeGlobalInitializers(const glslang::TIntermSequence&);
...@@ -3241,7 +3241,7 @@ bool TGlslangToSpvTraverser::isShaderEntryPoint(const glslang::TIntermAggregate* ...@@ -3241,7 +3241,7 @@ bool TGlslangToSpvTraverser::isShaderEntryPoint(const glslang::TIntermAggregate*
// Does parameter need a place to keep writes, separate from the original? // Does parameter need a place to keep writes, separate from the original?
// Assumes called after originalParam(), which filters out block/buffer/opaque-based // Assumes called after originalParam(), which filters out block/buffer/opaque-based
// qualifiers such that we should have only in/out/inout/constreadonly here. // qualifiers such that we should have only in/out/inout/constreadonly here.
bool TGlslangToSpvTraverser::writableParam(glslang::TStorageQualifier qualifier) bool TGlslangToSpvTraverser::writableParam(glslang::TStorageQualifier qualifier) const
{ {
assert(qualifier == glslang::EvqIn || assert(qualifier == glslang::EvqIn ||
qualifier == glslang::EvqOut || qualifier == glslang::EvqOut ||
...@@ -3954,18 +3954,17 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg ...@@ -3954,18 +3954,17 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg
// 3. Make the call // 3. Make the call
// 4. Copy back the results // 4. Copy back the results
// 1. Evaluate the arguments // 1. Evaluate the arguments and their types
std::vector<spv::Builder::AccessChain> lValues; std::vector<spv::Builder::AccessChain> lValues;
std::vector<spv::Id> rValues; std::vector<spv::Id> rValues;
std::vector<const glslang::TType*> argTypes; std::vector<const glslang::TType*> argTypes;
for (int a = 0; a < (int)glslangArgs.size(); ++a) { for (int a = 0; a < (int)glslangArgs.size(); ++a) {
const glslang::TType& paramType = glslangArgs[a]->getAsTyped()->getType(); argTypes.push_back(&glslangArgs[a]->getAsTyped()->getType());
// build l-value // build l-value
builder.clearAccessChain(); builder.clearAccessChain();
glslangArgs[a]->traverse(this); glslangArgs[a]->traverse(this);
argTypes.push_back(&paramType);
// keep outputs and pass-by-originals as l-values, evaluate others as r-values // keep outputs and pass-by-originals as l-values, evaluate others as r-values
if (originalParam(qualifiers[a], paramType, function->hasImplicitThis() && a == 0) || if (originalParam(qualifiers[a], *argTypes[a], function->hasImplicitThis() && a == 0) ||
writableParam(qualifiers[a])) { writableParam(qualifiers[a])) {
// save l-value // save l-value
lValues.push_back(builder.getAccessChain()); lValues.push_back(builder.getAccessChain());
...@@ -3983,26 +3982,33 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg ...@@ -3983,26 +3982,33 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg
int rValueCount = 0; int rValueCount = 0;
std::vector<spv::Id> spvArgs; std::vector<spv::Id> spvArgs;
for (int a = 0; a < (int)glslangArgs.size(); ++a) { for (int a = 0; a < (int)glslangArgs.size(); ++a) {
const glslang::TType& paramType = glslangArgs[a]->getAsTyped()->getType();
spv::Id arg; spv::Id arg;
if (originalParam(qualifiers[a], paramType, function->hasImplicitThis() && a == 0)) { if (originalParam(qualifiers[a], *argTypes[a], function->hasImplicitThis() && a == 0)) {
builder.setAccessChain(lValues[lValueCount]); builder.setAccessChain(lValues[lValueCount]);
arg = builder.accessChainGetLValue(); arg = builder.accessChainGetLValue();
++lValueCount; ++lValueCount;
} else if (writableParam(qualifiers[a])) { } else if (writableParam(qualifiers[a])) {
// need space to hold the copy // need space to hold the copy
arg = builder.createVariable(spv::StorageClassFunction, convertGlslangToSpvType(paramType), "param"); arg = builder.createVariable(spv::StorageClassFunction, builder.getContainedTypeId(function->getParamType(a)), "param");
if (qualifiers[a] == glslang::EvqIn || qualifiers[a] == glslang::EvqInOut) { if (qualifiers[a] == glslang::EvqIn || qualifiers[a] == glslang::EvqInOut) {
// need to copy the input into output space // need to copy the input into output space
builder.setAccessChain(lValues[lValueCount]); builder.setAccessChain(lValues[lValueCount]);
spv::Id copy = accessChainLoad(*argTypes[a]); spv::Id copy = accessChainLoad(*argTypes[a]);
builder.clearAccessChain(); builder.clearAccessChain();
builder.setAccessChainLValue(arg); builder.setAccessChainLValue(arg);
multiTypeStore(paramType, copy); multiTypeStore(*argTypes[a], copy);
} }
++lValueCount; ++lValueCount;
} else { } else {
arg = rValues[rValueCount]; // process r-value, which involves a copy for a type mismatch
if (function->getParamType(a) != convertGlslangToSpvType(*argTypes[a])) {
spv::Id argCopy = builder.createVariable(spv::StorageClassFunction, function->getParamType(a), "arg");
builder.clearAccessChain();
builder.setAccessChainLValue(argCopy);
multiTypeStore(*argTypes[a], rValues[rValueCount]);
arg = builder.createLoad(argCopy);
} else
arg = rValues[rValueCount];
++rValueCount; ++rValueCount;
} }
spvArgs.push_back(arg); spvArgs.push_back(arg);
...@@ -4015,14 +4021,13 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg ...@@ -4015,14 +4021,13 @@ spv::Id TGlslangToSpvTraverser::handleUserFunctionCall(const glslang::TIntermAgg
// 4. Copy back out an "out" arguments. // 4. Copy back out an "out" arguments.
lValueCount = 0; lValueCount = 0;
for (int a = 0; a < (int)glslangArgs.size(); ++a) { for (int a = 0; a < (int)glslangArgs.size(); ++a) {
const glslang::TType& paramType = glslangArgs[a]->getAsTyped()->getType(); if (originalParam(qualifiers[a], *argTypes[a], function->hasImplicitThis() && a == 0))
if (originalParam(qualifiers[a], paramType, function->hasImplicitThis() && a == 0))
++lValueCount; ++lValueCount;
else if (writableParam(qualifiers[a])) { else if (writableParam(qualifiers[a])) {
if (qualifiers[a] == glslang::EvqOut || qualifiers[a] == glslang::EvqInOut) { if (qualifiers[a] == glslang::EvqOut || qualifiers[a] == glslang::EvqInOut) {
spv::Id copy = builder.createLoad(spvArgs[a]); spv::Id copy = builder.createLoad(spvArgs[a]);
builder.setAccessChain(lValues[lValueCount]); builder.setAccessChain(lValues[lValueCount]);
multiTypeStore(paramType, copy); multiTypeStore(*argTypes[a], copy);
} }
++lValueCount; ++lValueCount;
} }
......
...@@ -258,7 +258,8 @@ public: ...@@ -258,7 +258,8 @@ public:
delete blocks[i]; delete blocks[i];
} }
Id getId() const { return functionInstruction.getResultId(); } Id getId() const { return functionInstruction.getResultId(); }
Id getParamId(int p) { return parameterInstructions[p]->getResultId(); } Id getParamId(int p) const { return parameterInstructions[p]->getResultId(); }
Id getParamType(int p) const { return parameterInstructions[p]->getTypeId(); }
void addBlock(Block* block) { blocks.push_back(block); } void addBlock(Block* block) { blocks.push_back(block); }
void removeBlock(Block* block) void removeBlock(Block* block)
......
spv.multiStructFuncall.frag spv.multiStructFuncall.frag
// Module Version 10000 // Module Version 10000
// Generated by (magic number): 80006 // Generated by (magic number): 80006
// Id's are bound by 63 // Id's are bound by 66
Capability Shader Capability Shader
1: ExtInstImport "GLSL.std.450" 1: ExtInstImport "GLSL.std.450"
...@@ -23,20 +23,21 @@ spv.multiStructFuncall.frag ...@@ -23,20 +23,21 @@ spv.multiStructFuncall.frag
Name 23 "blockName" Name 23 "blockName"
MemberName 23(blockName) 0 "s1" MemberName 23(blockName) 0 "s1"
Name 25 "" Name 25 ""
Name 33 "s2" Name 31 "S"
Name 36 "S" MemberName 31(S) 0 "m"
MemberName 36(S) 0 "m" Name 32 "arg"
Name 38 "param" Name 39 "s2"
Name 45 "param" Name 42 "param"
Name 48 "param" Name 48 "param"
Name 59 "param" Name 51 "param"
Name 62 "param"
MemberDecorate 22(S) 0 ColMajor MemberDecorate 22(S) 0 ColMajor
MemberDecorate 22(S) 0 Offset 0 MemberDecorate 22(S) 0 Offset 0
MemberDecorate 22(S) 0 MatrixStride 16 MemberDecorate 22(S) 0 MatrixStride 16
MemberDecorate 23(blockName) 0 Offset 0 MemberDecorate 23(blockName) 0 Offset 0
Decorate 23(blockName) BufferBlock Decorate 23(blockName) BufferBlock
Decorate 25 DescriptorSet 0 Decorate 25 DescriptorSet 0
MemberDecorate 36(S) 0 ColMajor MemberDecorate 31(S) 0 ColMajor
2: TypeVoid 2: TypeVoid
3: TypeFunction 2 3: TypeFunction 2
6: TypeFloat 32 6: TypeFloat 32
...@@ -53,48 +54,52 @@ spv.multiStructFuncall.frag ...@@ -53,48 +54,52 @@ spv.multiStructFuncall.frag
26: TypeInt 32 1 26: TypeInt 32 1
27: 26(int) Constant 0 27: 26(int) Constant 0
28: TypePointer Uniform 22(S) 28: TypePointer Uniform 22(S)
32: TypePointer Private 9(S) 31(S): TypeStruct 8
33(s2): 32(ptr) Variable Private 34: TypePointer Function 8
36(S): TypeStruct 8 38: TypePointer Private 9(S)
37: TypePointer Function 36(S) 39(s2): 38(ptr) Variable Private
42: TypePointer Function 8 60: TypePointer Uniform 8
57: TypePointer Uniform 8
4(main): 2 Function None 3 4(main): 2 Function None 3
5: Label 5: Label
38(param): 37(ptr) Variable Function 32(arg): 14(ptr) Variable Function
45(param): 14(ptr) Variable Function 42(param): 14(ptr) Variable Function
48(param): 37(ptr) Variable Function 48(param): 14(ptr) Variable Function
59(param): 14(ptr) Variable Function 51(param): 14(ptr) Variable Function
62(param): 14(ptr) Variable Function
29: 28(ptr) AccessChain 25 27 29: 28(ptr) AccessChain 25 27
30: 22(S) Load 29 30: 22(S) Load 29
31: 2 FunctionCall 12(fooConst(struct-S-mf441;) 30 33: 8 CompositeExtract 30 0
34: 9(S) Load 33(s2) 35: 34(ptr) AccessChain 32(arg) 27
35: 2 FunctionCall 12(fooConst(struct-S-mf441;) 34 Store 35 33
39: 28(ptr) AccessChain 25 27 36: 9(S) Load 32(arg)
40: 22(S) Load 39 37: 2 FunctionCall 12(fooConst(struct-S-mf441;) 36
41: 8 CompositeExtract 40 0 40: 9(S) Load 39(s2)
43: 42(ptr) AccessChain 38(param) 27 41: 2 FunctionCall 12(fooConst(struct-S-mf441;) 40
Store 43 41 43: 28(ptr) AccessChain 25 27
44: 2 FunctionCall 17(foo(struct-S-mf441;) 38(param) 44: 22(S) Load 43
46: 9(S) Load 33(s2) 45: 8 CompositeExtract 44 0
Store 45(param) 46 46: 34(ptr) AccessChain 42(param) 27
47: 2 FunctionCall 17(foo(struct-S-mf441;) 45(param) Store 46 45
49: 28(ptr) AccessChain 25 27 47: 2 FunctionCall 17(foo(struct-S-mf441;) 42(param)
50: 22(S) Load 49 49: 9(S) Load 39(s2)
51: 8 CompositeExtract 50 0 Store 48(param) 49
52: 42(ptr) AccessChain 48(param) 27 50: 2 FunctionCall 17(foo(struct-S-mf441;) 48(param)
Store 52 51 52: 28(ptr) AccessChain 25 27
53: 2 FunctionCall 20(fooOut(struct-S-mf441;) 48(param) 53: 22(S) Load 52
54: 36(S) Load 48(param) 54: 8 CompositeExtract 53 0
55: 28(ptr) AccessChain 25 27 55: 34(ptr) AccessChain 51(param) 27
56: 8 CompositeExtract 54 0 Store 55 54
58: 57(ptr) AccessChain 55 27 56: 2 FunctionCall 20(fooOut(struct-S-mf441;) 51(param)
Store 58 56 57: 9(S) Load 51(param)
60: 9(S) Load 33(s2) 58: 28(ptr) AccessChain 25 27
Store 59(param) 60 59: 8 CompositeExtract 57 0
61: 2 FunctionCall 20(fooOut(struct-S-mf441;) 59(param) 61: 60(ptr) AccessChain 58 27
62: 9(S) Load 59(param) Store 61 59
Store 33(s2) 62 63: 9(S) Load 39(s2)
Store 62(param) 63
64: 2 FunctionCall 20(fooOut(struct-S-mf441;) 62(param)
65: 9(S) Load 62(param)
Store 39(s2) 65
Return Return
FunctionEnd FunctionEnd
12(fooConst(struct-S-mf441;): 2 Function None 10 12(fooConst(struct-S-mf441;): 2 Function None 10
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment