Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 6381d7f

Browse files
ShabbyXCommit Bot
ShabbyX
authored and
Commit Bot
committed
Vulkan: Avoid linear search in shader on varying location assignment
Glslang wrapper was trying to identify whether the varying is declared in each of the out/in shaders by looping through the macro symbols that need replacement. This change instead adds stage information to PackedVarying assigned when collecting varyings. Glslang wrapper then simply tests the bitfield for the stages of interest. Bug: angleproject:3571 Change-Id: I29614e3e62d7df88e413c1732ac04e24243f167a Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1954677 Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org> Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>
1 parent 3c75d24 commit 6381d7f

File tree

6 files changed

+89
-46
lines changed

6 files changed

+89
-46
lines changed

‎src/libANGLE/Program.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4339,13 +4339,18 @@ ProgramMergedVaryings Program::getMergedVaryings() const
43394339
{
43404340
if (shader)
43414341
{
4342+
ShaderType shaderType = shader->getType();
43424343
for (const sh::ShaderVariable &varying : shader->getOutputVaryings())
43434344
{
4344-
merged[varying.name].frontShader = &varying;
4345+
ProgramVaryingRef *ref = &merged[varying.name];
4346+
ref->frontShader = &varying;
4347+
ref->frontShaderStage = shaderType;
43454348
}
43464349
for (const sh::ShaderVariable &varying : shader->getInputVaryings())
43474350
{
4348-
merged[varying.name].backShader = &varying;
4351+
ProgramVaryingRef *ref = &merged[varying.name];
4352+
ref->backShader = &varying;
4353+
ref->backShaderStage = shaderType;
43494354
}
43504355
}
43514356
}

‎src/libANGLE/Program.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,8 @@ struct ProgramVaryingRef
569569

570570
const sh::ShaderVariable *frontShader = nullptr;
571571
const sh::ShaderVariable *backShader = nullptr;
572+
ShaderType frontShaderStage = ShaderType::InvalidEnum;
573+
ShaderType backShaderStage = ShaderType::InvalidEnum;
572574
};
573575

574576
using ProgramMergedVaryings = std::map<std::string, ProgramVaryingRef>;

‎src/libANGLE/VaryingPacking.cpp‎

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,41 @@ bool ComparePackedVarying(const PackedVarying &x, const PackedVarying &y)
5555

5656
} // anonymous namespace
5757

58+
// Implementation of PackedVarying
59+
PackedVarying::PackedVarying(const sh::ShaderVariable &varyingIn,
60+
sh::InterpolationType interpolationIn)
61+
: PackedVarying(varyingIn, interpolationIn, "", false)
62+
{}
63+
PackedVarying::PackedVarying(const sh::ShaderVariable &varyingIn,
64+
sh::InterpolationType interpolationIn,
65+
const std::string &parentStructNameIn,
66+
GLuint fieldIndexIn)
67+
: varying(&varyingIn),
68+
interpolation(interpolationIn),
69+
parentStructName(parentStructNameIn),
70+
arrayIndex(GL_INVALID_INDEX),
71+
fieldIndex(fieldIndexIn)
72+
{}
73+
74+
PackedVarying::~PackedVarying() = default;
75+
76+
PackedVarying::PackedVarying(PackedVarying &&other)
77+
: PackedVarying(*other.varying, other.interpolation)
78+
{
79+
*this = std::move(other);
80+
}
81+
PackedVarying &PackedVarying::operator=(PackedVarying &&other)
82+
{
83+
std::swap(varying, other.varying);
84+
std::swap(shaderStages, other.shaderStages);
85+
std::swap(interpolation, other.interpolation);
86+
std::swap(parentStructName, other.parentStructName);
87+
std::swap(arrayIndex, other.arrayIndex);
88+
std::swap(fieldIndex, other.fieldIndex);
89+
90+
return *this;
91+
}
92+
5893
// Implementation of VaryingPacking
5994
VaryingPacking::VaryingPacking(GLuint maxVaryingVectors, PackMode packMode)
6095
: mRegisterMap(maxVaryingVectors), mPackMode(packMode)
@@ -295,6 +330,10 @@ bool VaryingPacking::collectAndPackUserVaryings(gl::InfoLog &infoLog,
295330
mInputVaryings.emplace_back(*input);
296331
}
297332

333+
ShaderBitSet shaderStages;
334+
shaderStages.set(ref.second.frontShaderStage);
335+
shaderStages.set(ref.second.backShaderStage);
336+
298337
// Only pack statically used varyings that have a matched input or output, plus special
299338
// builtins. Note that we pack all statically used user-defined varyings even if they are
300339
// not active. GLES specs are a bit vague on whether it's allowed to only pack active
@@ -326,19 +365,21 @@ bool VaryingPacking::collectAndPackUserVaryings(gl::InfoLog &infoLog,
326365
ASSERT(!field.isStruct() && !field.isArray());
327366
mPackedVaryings.emplace_back(field, interpolation, varying->name,
328367
fieldIndex);
368+
mPackedVaryings.back().shaderStages = shaderStages;
329369
uniqueFullNames.insert(mPackedVaryings.back().fullName());
330370
}
331371
}
332372
else
333373
{
334374
mPackedVaryings.emplace_back(*varying, interpolation);
375+
mPackedVaryings.back().shaderStages = shaderStages;
335376
uniqueFullNames.insert(mPackedVaryings.back().fullName());
336377
}
337378
continue;
338379
}
339380
}
340381

341-
// If the varying is not used in the VS, we know it is inactive.
382+
// If the varying is not used in the input, we know it is inactive.
342383
if (!input)
343384
{
344385
mInactiveVaryingNames.push_back(ref.first);
@@ -370,7 +411,7 @@ bool VaryingPacking::collectAndPackUserVaryings(gl::InfoLog &infoLog,
370411
ASSERT(!field->isStruct() && !field->isArray());
371412
mPackedVaryings.emplace_back(*field, input->interpolation, input->name,
372413
fieldIndex);
373-
mPackedVaryings.back().vertexOnly = true;
414+
mPackedVaryings.back().shaderStages.set(ShaderType::Vertex);
374415
mPackedVaryings.back().arrayIndex = GL_INVALID_INDEX;
375416
uniqueFullNames.insert(tfVarying);
376417
}
@@ -383,7 +424,7 @@ bool VaryingPacking::collectAndPackUserVaryings(gl::InfoLog &infoLog,
383424
if (tfVarying.compare(0, 3, "gl_") != 0)
384425
{
385426
mPackedVaryings.emplace_back(*input, input->interpolation);
386-
mPackedVaryings.back().vertexOnly = true;
427+
mPackedVaryings.back().shaderStages.set(ShaderType::Vertex);
387428
mPackedVaryings.back().arrayIndex = static_cast<GLuint>(subscript);
388429
uniqueFullNames.insert(tfVarying);
389430
}

‎src/libANGLE/VaryingPacking.h‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "angle_gl.h"
1818
#include "common/angleutils.h"
19+
#include "libANGLE/angletypes.h"
1920

2021
#include <map>
2122

@@ -26,22 +27,17 @@ struct ProgramVaryingRef;
2627

2728
using ProgramMergedVaryings = std::map<std::string, ProgramVaryingRef>;
2829

29-
struct PackedVarying
30+
struct PackedVarying : angle::NonCopyable
3031
{
31-
PackedVarying(const sh::ShaderVariable &varyingIn, sh::InterpolationType interpolationIn)
32-
: PackedVarying(varyingIn, interpolationIn, "", false)
33-
{}
32+
PackedVarying(const sh::ShaderVariable &varyingIn, sh::InterpolationType interpolationIn);
3433
PackedVarying(const sh::ShaderVariable &varyingIn,
3534
sh::InterpolationType interpolationIn,
3635
const std::string &parentStructNameIn,
37-
GLuint fieldIndexIn)
38-
: varying(&varyingIn),
39-
vertexOnly(false),
40-
interpolation(interpolationIn),
41-
parentStructName(parentStructNameIn),
42-
arrayIndex(GL_INVALID_INDEX),
43-
fieldIndex(fieldIndexIn)
44-
{}
36+
GLuint fieldIndexIn);
37+
PackedVarying(PackedVarying &&other);
38+
~PackedVarying();
39+
40+
PackedVarying &operator=(PackedVarying &&other);
4541

4642
bool isStructField() const { return !parentStructName.empty(); }
4743

@@ -63,10 +59,17 @@ struct PackedVarying
6359
return fullNameStr.str();
6460
}
6561

62+
// Transform feedback varyings can be only referenced in the VS.
63+
bool vertexOnly() const
64+
{
65+
ShaderBitSet vertex;
66+
vertex.set(ShaderType::Vertex);
67+
return shaderStages == vertex;
68+
}
69+
6670
const sh::ShaderVariable *varying;
6771

68-
// Transform feedback varyings can be only referenced in the VS.
69-
bool vertexOnly;
72+
ShaderBitSet shaderStages;
7073

7174
// Cached so we can store sh::ShaderVariable to point to varying fields.
7275
sh::InterpolationType interpolation;

‎src/libANGLE/renderer/d3d/DynamicHLSL.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -803,7 +803,7 @@ void DynamicHLSL::generateShaderLinkHLSL(const gl::Caps &caps,
803803

804804
// Don't reference VS-only transform feedback varyings in the PS. Note that we're relying on
805805
// that the active flag is set according to usage in the fragment shader.
806-
if (packedVarying.vertexOnly || !varying.active)
806+
if (packedVarying.vertexOnly() || !varying.active)
807807
continue;
808808

809809
pixelPrologue << " ";

‎src/libANGLE/renderer/glslang_wrapper_utils.cpp‎

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ class IntermediateShaderSource final : angle::NonCopyable
132132
void init(const std::string &source);
133133
bool empty() const { return mTokens.empty(); }
134134

135-
bool findTokenName(const std::string &name);
136135
// Find @@ LAYOUT-name(extra, args) @@ and replace it with:
137136
//
138137
// layout(specifier, extra, args)
@@ -311,18 +310,6 @@ void IntermediateShaderSource::init(const std::string &source)
311310
}
312311
}
313312

314-
bool IntermediateShaderSource::findTokenName(const std::string &name)
315-
{
316-
for (Token &block : mTokens)
317-
{
318-
if (block.text == name)
319-
{
320-
return true;
321-
}
322-
}
323-
return false;
324-
}
325-
326313
void IntermediateShaderSource::insertLayoutSpecifier(const std::string &name,
327314
const std::string &specifier)
328315
{
@@ -728,10 +715,15 @@ void AssignOutputLocations(const gl::ProgramState &programState,
728715

729716
void AssignVaryingLocations(const gl::ProgramState &programState,
730717
const gl::ProgramLinkedResources &resources,
731-
IntermediateShaderSource *outStageSource,
732-
IntermediateShaderSource *inStageSource,
718+
gl::ShaderType outStage,
719+
gl::ShaderType inStage,
720+
gl::ShaderMap<IntermediateShaderSource> *shaderSources,
733721
XfbBufferMap *xfbBufferMap)
734722
{
723+
724+
IntermediateShaderSource *outStageSource = &(*shaderSources)[outStage];
725+
IntermediateShaderSource *inStageSource = &(*shaderSources)[inStage];
726+
735727
// Assign varying locations.
736728
for (const gl::PackedVaryingRegister &varyingReg : resources.varyingPacking.getRegisterList())
737729
{
@@ -768,13 +760,13 @@ void AssignVaryingLocations(const gl::ProgramState &programState,
768760
const std::string &name =
769761
varying.isStructField() ? varying.parentStructName : varying.varying->name;
770762

771-
// Varings are from 3 stage of shader sources
763+
// Varyings are from multiple shader stages
772764
// To match pair of (out - in) qualifier, varying should be in the pair of shader source
773-
if (!outStageSource->findTokenName(name) || !inStageSource->findTokenName(name))
765+
if (!varying.shaderStages.test(outStage) || !varying.shaderStages.test(inStage))
774766
{
775767
// Pair can be unmatching at transform feedback case,
776768
// But it requires qualifier.
777-
if (!varying.vertexOnly)
769+
if (!varying.vertexOnly())
778770
continue;
779771
}
780772

@@ -1262,28 +1254,28 @@ void GlslangGetShaderSource(const GlslangSourceOptions &options,
12621254
if (!geometrySource->empty())
12631255
{
12641256
AssignOutputLocations(programState, fragmentSource);
1265-
AssignVaryingLocations(programState, resources, geometrySource, fragmentSource,
1266-
&xfbBufferMap);
1257+
AssignVaryingLocations(programState, resources, gl::ShaderType::Geometry,
1258+
gl::ShaderType::Fragment, &intermediateSources, &xfbBufferMap);
12671259
if (!vertexSource->empty())
12681260
{
12691261
AssignAttributeLocations(programState, vertexSource);
1270-
AssignVaryingLocations(programState, resources, vertexSource, geometrySource,
1271-
&xfbBufferMap);
1262+
AssignVaryingLocations(programState, resources, gl::ShaderType::Vertex,
1263+
gl::ShaderType::Geometry, &intermediateSources, &xfbBufferMap);
12721264
}
12731265
}
12741266
else if (!vertexSource->empty())
12751267
{
12761268
AssignAttributeLocations(programState, vertexSource);
12771269
AssignOutputLocations(programState, fragmentSource);
1278-
AssignVaryingLocations(programState, resources, vertexSource, fragmentSource,
1279-
&xfbBufferMap);
1270+
AssignVaryingLocations(programState, resources, gl::ShaderType::Vertex,
1271+
gl::ShaderType::Fragment, &intermediateSources, &xfbBufferMap);
12801272
}
12811273
else if (!fragmentSource->empty())
12821274
{
12831275
AssignAttributeLocations(programState, fragmentSource);
12841276
AssignOutputLocations(programState, fragmentSource);
1285-
AssignVaryingLocations(programState, resources, vertexSource, fragmentSource,
1286-
&xfbBufferMap);
1277+
AssignVaryingLocations(programState, resources, gl::ShaderType::Vertex,
1278+
gl::ShaderType::Fragment, &intermediateSources, &xfbBufferMap);
12871279
}
12881280
AssignUniformBindings(options, &intermediateSources);
12891281
AssignTextureBindings(options, useOldRewriteStructSamplers, programState, &intermediateSources);

0 commit comments

Comments
 (0)