Re: [Mesa-dev] [PATCH 0/6] Fix Various Compilation Issues With Bindless
On Sat, Jun 30, 2018 at 10:56 PM, Karol Herbst wrote: > On Mon, Jun 11, 2018 at 5:10 PM, Marek Olšák wrote: >> The series is OK with me, even though radeonsi can't support the new >> opcodes. >> > > How would you handle the case when a local variable might get a > bindless or a non bindless sampler value assigned? Meaning, how can a > compliant implementation of bindless_texture then look like on > radeonsi? radeonsi is designed such that nonbindless and bindless arrays of binding slots are separate. You either get a nonbindless index within nonbindless per-shader slots, or you get a bindless handle that is an index into bindless slots. We could merge those arrays of binding slots into one, though at the cost of added CPU overhead. So we are not going to do that. This is one of the weird things that get added into the spec even though they are unnecessary. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/6] Fix Various Compilation Issues With Bindless
On Mon, Jun 11, 2018 at 5:10 PM, Marek Olšák wrote: > The series is OK with me, even though radeonsi can't support the new > opcodes. > How would you handle the case when a local variable might get a bindless or a non bindless sampler value assigned? Meaning, how can a compliant implementation of bindless_texture then look like on radeonsi? > Marek > > On Mon, Jun 11, 2018 at 6:24 AM, Rhys Perry > wrote: >> >> Ping to those who seem appropriate for this patch in case it was forgotten >> or missed. > > > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nv50/ir: handle clipvertex for geometry shaders as well
On Sat, Jun 30, 2018 at 4:05 PM, Karol Herbst wrote: > On Sat, Jun 30, 2018 at 9:30 PM, Ilia Mirkin wrote: >> Tes too, right? Also does the logic that forces recompiles work ok? I seem >> to recall it was tied to vs. >> > > well, I didn't want to touch stuff where we don't have a piglit test > yet, so everything else is untested. And as we don't start to expose a > compatibility profile yet I think it would be fine to do little steps > for now. I am sure that this works as far as piglit goes. Sounds like we need the following piglits: (a) UCP's or gl_ClipVertex usage in a TES (b) For both GS and TES, adjusting the number of enabled clip planes up and down (starting low, going high, and then going back down again). -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nv50/ir: handle clipvertex for geometry shaders as well
On Sat, Jun 30, 2018 at 9:30 PM, Ilia Mirkin wrote:
> Tes too, right? Also does the logic that forces recompiles work ok? I seem
> to recall it was tied to vs.
>
well, I didn't want to touch stuff where we don't have a piglit test
yet, so everything else is untested. And as we don't start to expose a
compatibility profile yet I think it would be fine to do little steps
for now. I am sure that this works as far as piglit goes.
> On Sat, Jun 30, 2018, 10:18 Karol Herbst wrote:
>>
>> this will be needed for compatibility profiles
>>
>> Signed-off-by: Karol Herbst
>> ---
>> src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 5 -
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> index c92acc996c4..1151e0ee255 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
>> @@ -3613,6 +3613,9 @@ Converter::handleInstruction(const struct
>> tgsi_full_instruction *insn)
>>info->out[info->io.viewportId].slot[0]
>> * 4);
>> mkStore(OP_EXPORT, TYPE_U32, vpSym, NULL, viewport);
>>}
>> + /* handle user clip planes for each emitted vertex */
>> + if (info->io.genUserClip > 0)
>> + handleUserClipPlanes();
>>/* fallthrough */
>> case TGSI_OPCODE_ENDPRIM:
>> {
>> @@ -3787,7 +3790,7 @@ Converter::handleInstruction(const struct
>> tgsi_full_instruction *insn)
>>setPosition(epilogue, true);
>>if (prog->getType() == Program::TYPE_FRAGMENT)
>> exportOutputs();
>> - if (info->io.genUserClip > 0)
>> + if (prog->getType() == Program::TYPE_VERTEX && info->io.genUserClip
>> > 0)
>> handleUserClipPlanes();
>>mkOp(OP_EXIT, TYPE_NONE, NULL)->terminator = 1;
>> }
>> --
>> 2.17.1
>>
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nv50/ir: handle clipvertex for geometry shaders as well
Tes too, right? Also does the logic that forces recompiles work ok? I seem
to recall it was tied to vs.
On Sat, Jun 30, 2018, 10:18 Karol Herbst wrote:
> this will be needed for compatibility profiles
>
> Signed-off-by: Karol Herbst
> ---
> src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> index c92acc996c4..1151e0ee255 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> @@ -3613,6 +3613,9 @@ Converter::handleInstruction(const struct
> tgsi_full_instruction *insn)
>info->out[info->io.viewportId].slot[0]
> * 4);
> mkStore(OP_EXPORT, TYPE_U32, vpSym, NULL, viewport);
>}
> + /* handle user clip planes for each emitted vertex */
> + if (info->io.genUserClip > 0)
> + handleUserClipPlanes();
>/* fallthrough */
> case TGSI_OPCODE_ENDPRIM:
> {
> @@ -3787,7 +3790,7 @@ Converter::handleInstruction(const struct
> tgsi_full_instruction *insn)
>setPosition(epilogue, true);
>if (prog->getType() == Program::TYPE_FRAGMENT)
> exportOutputs();
> - if (info->io.genUserClip > 0)
> + if (prog->getType() == Program::TYPE_VERTEX && info->io.genUserClip
> > 0)
> handleUserClipPlanes();
>mkOp(OP_EXIT, TYPE_NONE, NULL)->terminator = 1;
> }
> --
> 2.17.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] swr/rast: Rename createInstructionSimplifierPass with llvm-7.0.
Fix build error after llvm-7.0svn r336028 ("[instsimplify] Move the
instsimplify pass to use more obvious file names and diretory.").
rasterizer/jitter/blend_jit.cpp:873:20: error: use of undeclared identifier
'createInstructionSimplifierPass'
passes.add(createInstructionSimplifierPass());
^
Signed-off-by: Vinson Lee
---
src/gallium/drivers/swr/rasterizer/jitter/blend_jit.cpp | 4
src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp | 4
src/gallium/drivers/swr/rasterizer/jitter/jit_pch.hpp | 1 +
src/gallium/drivers/swr/rasterizer/jitter/streamout_jit.cpp | 4
4 files changed, 13 insertions(+)
diff --git a/src/gallium/drivers/swr/rasterizer/jitter/blend_jit.cpp
b/src/gallium/drivers/swr/rasterizer/jitter/blend_jit.cpp
index f89c502db7d7..1d6fb405dd6b 100644
--- a/src/gallium/drivers/swr/rasterizer/jitter/blend_jit.cpp
+++ b/src/gallium/drivers/swr/rasterizer/jitter/blend_jit.cpp
@@ -870,7 +870,11 @@ struct BlendJit : public Builder
passes.add(createCFGSimplificationPass());
passes.add(createEarlyCSEPass());
passes.add(createInstructionCombiningPass());
+#if LLVM_VERSION_MAJOR >= 7
+passes.add(createInstSimplifyLegacyPass());
+#else
passes.add(createInstructionSimplifierPass());
+#endif
passes.add(createConstantPropagationPass());
passes.add(createSCCPPass());
passes.add(createAggressiveDCEPass());
diff --git a/src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp
b/src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp
index b4d326ebdcc2..26972cddc251 100644
--- a/src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp
+++ b/src/gallium/drivers/swr/rasterizer/jitter/fetch_jit.cpp
@@ -294,7 +294,11 @@ Function* FetchJit::Create(const FETCH_COMPILE_STATE&
fetchState)
optPasses.add(createCFGSimplificationPass());
optPasses.add(createEarlyCSEPass());
optPasses.add(createInstructionCombiningPass());
+#if LLVM_VERSION_MAJOR >= 7
+optPasses.add(createInstSimplifyLegacyPass());
+#else
optPasses.add(createInstructionSimplifierPass());
+#endif
optPasses.add(createConstantPropagationPass());
optPasses.add(createSCCPPass());
optPasses.add(createAggressiveDCEPass());
diff --git a/src/gallium/drivers/swr/rasterizer/jitter/jit_pch.hpp
b/src/gallium/drivers/swr/rasterizer/jitter/jit_pch.hpp
index 47f717bfc2a1..760d1dab80ee 100644
--- a/src/gallium/drivers/swr/rasterizer/jitter/jit_pch.hpp
+++ b/src/gallium/drivers/swr/rasterizer/jitter/jit_pch.hpp
@@ -70,6 +70,7 @@ using PassManager = llvm::legacy::PassManager;
#if LLVM_VERSION_MAJOR >= 7
#include "llvm/Transforms/Utils.h"
#include "llvm/Transforms/InstCombine/InstCombine.h"
+#include "llvm/Transforms/Scalar/InstSimplifyPass.h"
#endif
#include "llvm/Support/Host.h"
#include "llvm/Support/DynamicLibrary.h"
diff --git a/src/gallium/drivers/swr/rasterizer/jitter/streamout_jit.cpp
b/src/gallium/drivers/swr/rasterizer/jitter/streamout_jit.cpp
index 8f86af2a4b41..5c1555280fce 100644
--- a/src/gallium/drivers/swr/rasterizer/jitter/streamout_jit.cpp
+++ b/src/gallium/drivers/swr/rasterizer/jitter/streamout_jit.cpp
@@ -306,7 +306,11 @@ struct StreamOutJit : public Builder
passes.add(createCFGSimplificationPass());
passes.add(createEarlyCSEPass());
passes.add(createInstructionCombiningPass());
+#if LLVM_VERSION_MAJOR >= 7
+passes.add(createInstSimplifyLegacyPass());
+#else
passes.add(createInstructionSimplifierPass());
+#endif
passes.add(createConstantPropagationPass());
passes.add(createSCCPPass());
passes.add(createAggressiveDCEPass());
--
2.18.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nv50/ir: handle clipvertex for geometry shaders as well
this will be needed for compatibility profiles
Signed-off-by: Karol Herbst
---
src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
index c92acc996c4..1151e0ee255 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -3613,6 +3613,9 @@ Converter::handleInstruction(const struct
tgsi_full_instruction *insn)
info->out[info->io.viewportId].slot[0] * 4);
mkStore(OP_EXPORT, TYPE_U32, vpSym, NULL, viewport);
}
+ /* handle user clip planes for each emitted vertex */
+ if (info->io.genUserClip > 0)
+ handleUserClipPlanes();
/* fallthrough */
case TGSI_OPCODE_ENDPRIM:
{
@@ -3787,7 +3790,7 @@ Converter::handleInstruction(const struct
tgsi_full_instruction *insn)
setPosition(epilogue, true);
if (prog->getType() == Program::TYPE_FRAGMENT)
exportOutputs();
- if (info->io.genUserClip > 0)
+ if (prog->getType() == Program::TYPE_VERTEX && info->io.genUserClip > 0)
handleUserClipPlanes();
mkOp(OP_EXIT, TYPE_NONE, NULL)->terminator = 1;
}
--
2.17.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 0/2] r600: Fix array texture slice index evaluation
it turned out that the failures with GATHER*O were the result of a faulty optimization done by sb. So for consistensy I'll also add the offset for GATHER4_O and GATHER4_C_O in the SET_TEXTURE_OFFSET call which alleviates the problems with sb. Apart from that I've just fixed some comments following Roland's remarks. I ran the dEQP GLES 2, 3, and 31 suites and piglit run gpu -t texture -t gather I didn't see any regressions, but textureSize 140 tes sampler2DArrayShadow -auto -fbo seems to be unstable. It usually passes, but once in a while it fails (on both, master and with this patch applied) Best, Gert Gert Wollny (2): r600: correct texture offset for array index lookup r600: set rounding mode for texture array layer selection src/gallium/drivers/r600/evergreen_state.c | 23 +++ src/gallium/drivers/r600/r600_shader.c | 29 +++-- 2 files changed, 50 insertions(+), 2 deletions(-) -- 2.16.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 2/2] r600: set rounding mode for texture array layer selection
From: Gert Wollny
The evaluation of the array layer index is "floor(z+0.5)", and the default
rounding mode doesn't correctly evaluate this. Therefore, set the rounding
mode to "trunc" and and z-filter mode to "point".
For other textures make sure the the default rounding mode and z-filter are
used.
Fixes single-sample tests out of:
dEQP-GLES3.functional.texture.shadow.2d_array.*
dEQP-GLES3.functional.texture.vertex.2d_array.*
dEQP-GLES3.functional.texture.filtering.2d_array.*
(With the single sample tests the rounding accuracy is tested too)
v2: * reword comments and commit message
* clear S_03C008_TRUNC_COORD for all non-array types
v2: reword comments
v3: correct typos and add comment about impact of using TRUNC on all
coordinates using POINT interpolation.
Acked-by: Roland Scheidegger (v2)
Signed-off-by: Gert Wollny
---
src/gallium/drivers/r600/evergreen_state.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/src/gallium/drivers/r600/evergreen_state.c
b/src/gallium/drivers/r600/evergreen_state.c
index a484f0078a..5e5e3a0324 100644
--- a/src/gallium/drivers/r600/evergreen_state.c
+++ b/src/gallium/drivers/r600/evergreen_state.c
@@ -2413,6 +2413,29 @@ static void evergreen_emit_sampler_states(struct
r600_context *rctx,
rstate = texinfo->states.states[i];
assert(rstate);
+ /* For texture arrays the formula to select the layer is
(floor(z + 0.5)) and
+* apparently the hardware doesn't trigger this when the
texture is in ARRAY mode
+* Neither does the default z-rounding mode provide the
required 0.5 shift
+* nor does it round with sufficient accuracy. Consequently set
the coordinate
+* interpolation and truncate mode here to get "floor" for
positive coordinates.
+* (Note, that this changes the rounding mode for all
coordinated in POINT
+* interpolation mode. Adding the 0.5 offset is done in the
shader.
+* Also make sure that for other texture types the default is
used.
+*/
+ struct r600_pipe_sampler_view *rview =
texinfo->views.views[i];
+ if (rview) {
+ rstate->tex_sampler_words[0] &= C_03C000_Z_FILTER;
+ enum pipe_texture_target target =
rview->base.texture->target;
+ if (target == PIPE_TEXTURE_2D_ARRAY ||
+ target == PIPE_TEXTURE_CUBE_ARRAY ||
+ target == PIPE_TEXTURE_1D_ARRAY) {
+ rstate->tex_sampler_words[0] |=
S_03C000_Z_FILTER(V_03C000_SQ_TEX_Z_FILTER_POINT);
+ rstate->tex_sampler_words[2] |=
S_03C008_TRUNCATE_COORD(1);
+ } else {
+ rstate->tex_sampler_words[2] &=
C_03C008_TRUNCATE_COORD;
+ }
+ }
+
radeon_emit(cs, PKT3(PKT3_SET_SAMPLER, 3, 0) | pkt_flags);
radeon_emit(cs, (resource_id_base + i) * 3);
radeon_emit_array(cs, rstate->tex_sampler_words, 3);
--
2.16.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 1/2] r600: correct texture offset for array index lookup
From: Gert Wollny
For texture array lookup the slice index is evaluated according to
idx = floor(z + 0.5)
This patch implements the first part by adding 0.5 to the according
texture coordinate when appropriate.
Fixes multi-sample tests out of:
dEQP-GLES3.functional.texture.shadow.2d_array.*
dEQP-GLES3.functional.texture.vertex.2d_array.*
dEQP-GLES3.functional.texture.filtering.2d_array.*
(In the multi-sample case the rounding accuracy is not tested.)
v2: - Don't apply texture offset correction for GATHER*O (corrects piglit
failures reported by Dave Airlie)
- unconditionally set the texture offset to 1 (=0.5) because the shader
can't set an offset for the array index (Roland Scheidegger)
v3: - Set texture offset also for GATHER*0 via SET_TEXTURE_OFFSET to be
consistent for all GATHER operations (thanks Roland Scheidegger for
pointing out this inconsistency).
- Don't set the offset for GET_TEXTURE_RESINFO operations
- correct typos (Roland)
Acked-by: Roland Scheidegger (v2)
Signed-off-by: Gert Wollny
---
src/gallium/drivers/r600/r600_shader.c | 29 +++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/src/gallium/drivers/r600/r600_shader.c
b/src/gallium/drivers/r600/r600_shader.c
index c466a48262..ffc272cf6f 100644
--- a/src/gallium/drivers/r600/r600_shader.c
+++ b/src/gallium/drivers/r600/r600_shader.c
@@ -7456,6 +7456,7 @@ static int tgsi_tex(struct r600_shader_ctx *ctx)
int8_t offset_x = 0, offset_y = 0, offset_z = 0;
boolean has_txq_cube_array_z = false;
unsigned sampler_index_mode;
+ int *array_index_offset = NULL;
if (inst->Instruction.Opcode == TGSI_OPCODE_TXQ &&
((inst->Texture.Texture == TGSI_TEXTURE_CUBE_ARRAY ||
@@ -8251,7 +8252,15 @@ static int tgsi_tex(struct r600_shader_ctx *ctx)
tex.src_gpr = ctx->file_offset[inst->TexOffsets[0].File] +
inst->TexOffsets[0].Index;
tex.src_sel_x = inst->TexOffsets[0].SwizzleX;
tex.src_sel_y = inst->TexOffsets[0].SwizzleY;
- tex.src_sel_z = inst->TexOffsets[0].SwizzleZ;
+
+ if (inst->Texture.Texture == TGSI_TEXTURE_2D_ARRAY ||
+ inst->Texture.Texture == TGSI_TEXTURE_SHADOW2D_ARRAY ||
+ inst->Texture.Texture == TGSI_TEXTURE_CUBE_ARRAY ||
+ inst->Texture.Texture == TGSI_TEXTURE_SHADOWCUBE_ARRAY)
+ tex.src_sel_z = PIPE_SWIZZLE_1;
+ else
+ tex.src_sel_z = inst->TexOffsets[0].SwizzleZ;
+
tex.src_sel_w = 4;
tex.dst_sel_x = 7;
@@ -8411,18 +8420,34 @@ static int tgsi_tex(struct r600_shader_ctx *ctx)
opcode == FETCH_OP_SAMPLE_C_LB) {
/* the array index is read from Y */
tex.coord_type_y = 0;
+ array_index_offset = _y;
} else {
/* the array index is read from Z */
tex.coord_type_z = 0;
tex.src_sel_z = tex.src_sel_y;
+ array_index_offset = _z;
+
}
} else if (inst->Texture.Texture == TGSI_TEXTURE_2D_ARRAY ||
inst->Texture.Texture == TGSI_TEXTURE_SHADOW2D_ARRAY ||
((inst->Texture.Texture == TGSI_TEXTURE_CUBE_ARRAY ||
inst->Texture.Texture == TGSI_TEXTURE_SHADOWCUBE_ARRAY) &&
- (ctx->bc->chip_class >= EVERGREEN)))
+ (ctx->bc->chip_class >= EVERGREEN))) {
/* the array index is read from Z */
tex.coord_type_z = 0;
+ array_index_offset = _z;
+ }
+
+ /* We have array access, the coordinates are not int and we use the
+* offset registers -> add 0.5 to the array index to adjust it according
+* to floor(z + 0.5). The floor operation is set as TRUNC in the texture
+* state.
+*/
+ if (array_index_offset &&
+ opcode != FETCH_OP_LD && opcode != FETCH_OP_GET_TEXTURE_RESINFO &&
+ opcode != FETCH_OP_GATHER4_C_O && opcode != FETCH_OP_GATHER4_O) {
+ *array_index_offset = 1;
+ }
/* mask unused source components */
if (opcode == FETCH_OP_SAMPLE || opcode == FETCH_OP_GATHER4) {
--
2.16.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 16/18] mesa/glspirv: lower workgroup access to offsets
Reviewed-by: Timothy Arceri
On 30/06/18 00:29, Alejandro Piñeiro wrote:
From: Antia Puentes
This will perform the CS shared lowering. See 8761a04d0d93
---
src/mesa/main/glspirv.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/mesa/main/glspirv.c b/src/mesa/main/glspirv.c
index c585bc51bbf..8ad6c373914 100644
--- a/src/mesa/main/glspirv.c
+++ b/src/mesa/main/glspirv.c
@@ -206,6 +206,7 @@ _mesa_spirv_to_nir(struct gl_context *ctx,
}
const struct spirv_to_nir_options spirv_options = {
+ .lower_workgroup_access_to_offsets = true,
.caps = ctx->Const.SpirVCapabilities
};
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 18/18] i965: Use the new nir atomic counter linker for SPIR-V shaders
Reviewed-by: Timothy Arceri
On 30/06/18 00:29, Alejandro Piñeiro wrote:
From: Neil Roberts
---
src/mesa/drivers/dri/i965/brw_link.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp
b/src/mesa/drivers/dri/i965/brw_link.cpp
index 8bc97fa4f3e..1071056f149 100644
--- a/src/mesa/drivers/dri/i965/brw_link.cpp
+++ b/src/mesa/drivers/dri/i965/brw_link.cpp
@@ -265,6 +265,8 @@ brw_link_shader(struct gl_context *ctx, struct
gl_shader_program *shProg)
if (shProg->data->spirv) {
if (!gl_nir_link_uniforms(ctx, shProg))
return false;
+
+ gl_nir_link_assign_atomic_counter_resources(ctx, shProg);
}
/* Determine first and last stage. */
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 17/18] i965: enable AtomicStorage capability for gen7+
Reviewed-by: Timothy Arceri On 30/06/18 00:29, Alejandro Piñeiro wrote: That is the same gen requirement for ARB_shader_atomic_counters. --- src/mesa/drivers/dri/i965/brw_context.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c index 9ced230ec14..e755de6241a 100644 --- a/src/mesa/drivers/dri/i965/brw_context.c +++ b/src/mesa/drivers/dri/i965/brw_context.c @@ -363,6 +363,7 @@ brw_initialize_spirv_supported_capabilities(struct brw_context *brw) ctx->Const.SpirVCapabilities.draw_parameters = true; ctx->Const.SpirVCapabilities.image_write_without_format = true; ctx->Const.SpirVCapabilities.variable_pointers = true; + ctx->Const.SpirVCapabilities.atomic_storage = devinfo->gen >= 7; } static void ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 15/18] nir: Fix OpAtomicCounterIDecrement for uniform atomic counters
On 30/06/18 00:29, Alejandro Piñeiro wrote:
From: Antia Puentes
From the SPIR-V specification, OpAtomicIDecrement:
"The instruction's result is the Original Value."
However, we were implementing it, for uniform atomic counters, as a
pre-decrement operation.
Renamed the former nir intrinsic 'atomic_counter_dec*' to
'atomic_counter_pre_dec*' for clarification purposes, as it implements
a pre-decrement operation as specified for OpenGL.
Added a new nir intrinsic 'atomic_counter_pos_dec*' which implements a
post-decrement operation as required by SPIR-V.
Can we have proper OpenGL and spriv quotes here please?
Also if we must have two intrinsics can we use the name:
atomic_counter_post_dec_deref
pos tends to be used as an abreviation for position.
---
src/compiler/glsl/gl_nir_lower_atomics.c | 8 ++--
src/compiler/glsl/glsl_to_nir.cpp| 4 ++--
src/compiler/nir/nir_intrinsics.py | 3 ++-
src/compiler/nir/nir_lower_atomics_to_ssbo.c | 8 +---
src/compiler/spirv/spirv_to_nir.c| 2 +-
5 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/src/compiler/glsl/gl_nir_lower_atomics.c
b/src/compiler/glsl/gl_nir_lower_atomics.c
index 293730966fd..73d058f3902 100644
--- a/src/compiler/glsl/gl_nir_lower_atomics.c
+++ b/src/compiler/glsl/gl_nir_lower_atomics.c
@@ -53,8 +53,12 @@ lower_deref_instr(nir_builder *b, nir_intrinsic_instr *instr,
op = nir_intrinsic_atomic_counter_inc;
break;
- case nir_intrinsic_atomic_counter_dec_deref:
- op = nir_intrinsic_atomic_counter_dec;
+ case nir_intrinsic_atomic_counter_pre_dec_deref:
+ op = nir_intrinsic_atomic_counter_pre_dec;
+ break;
+
+ case nir_intrinsic_atomic_counter_pos_dec_deref:
+ op = nir_intrinsic_atomic_counter_pos_dec;
break;
case nir_intrinsic_atomic_counter_add_deref:
diff --git a/src/compiler/glsl/glsl_to_nir.cpp
b/src/compiler/glsl/glsl_to_nir.cpp
index d3a3fb9b085..2d76c7e6cfe 100644
--- a/src/compiler/glsl/glsl_to_nir.cpp
+++ b/src/compiler/glsl/glsl_to_nir.cpp
@@ -630,7 +630,7 @@ nir_visitor::visit(ir_call *ir)
op = nir_intrinsic_atomic_counter_inc_deref;
break;
case ir_intrinsic_atomic_counter_predecrement:
- op = nir_intrinsic_atomic_counter_dec_deref;
+ op = nir_intrinsic_atomic_counter_pre_dec_deref;
break;
case ir_intrinsic_atomic_counter_add:
op = nir_intrinsic_atomic_counter_add_deref;
@@ -831,7 +831,7 @@ nir_visitor::visit(ir_call *ir)
switch (op) {
case nir_intrinsic_atomic_counter_read_deref:
case nir_intrinsic_atomic_counter_inc_deref:
- case nir_intrinsic_atomic_counter_dec_deref:
+ case nir_intrinsic_atomic_counter_pre_dec_deref:
case nir_intrinsic_atomic_counter_add_deref:
case nir_intrinsic_atomic_counter_min_deref:
case nir_intrinsic_atomic_counter_max_deref:
diff --git a/src/compiler/nir/nir_intrinsics.py
b/src/compiler/nir/nir_intrinsics.py
index d9d0bbdfccf..c5cbc09c598 100644
--- a/src/compiler/nir/nir_intrinsics.py
+++ b/src/compiler/nir/nir_intrinsics.py
@@ -270,7 +270,8 @@ def atomic3(name):
intrinsic(name, src_comp=[1, 1, 1], dest_comp=1, indices=[BASE])
atomic("atomic_counter_inc")
-atomic("atomic_counter_dec")
+atomic("atomic_counter_pre_dec")
+atomic("atomic_counter_pos_dec")
atomic("atomic_counter_read", flags=[CAN_ELIMINATE])
atomic2("atomic_counter_add")
atomic2("atomic_counter_min")
diff --git a/src/compiler/nir/nir_lower_atomics_to_ssbo.c
b/src/compiler/nir/nir_lower_atomics_to_ssbo.c
index 934ae81d750..34ac9fce40a 100644
--- a/src/compiler/nir/nir_lower_atomics_to_ssbo.c
+++ b/src/compiler/nir/nir_lower_atomics_to_ssbo.c
@@ -71,7 +71,8 @@ lower_instr(nir_intrinsic_instr *instr, unsigned ssbo_offset,
nir_builder *b)
return true;
case nir_intrinsic_atomic_counter_inc:
case nir_intrinsic_atomic_counter_add:
- case nir_intrinsic_atomic_counter_dec:
+ case nir_intrinsic_atomic_counter_pre_dec:
+ case nir_intrinsic_atomic_counter_pos_dec:
/* inc and dec get remapped to add: */
op = nir_intrinsic_ssbo_atomic_add;
break;
@@ -119,7 +120,8 @@ lower_instr(nir_intrinsic_instr *instr, unsigned
ssbo_offset, nir_builder *b)
nir_src_copy(_instr->src[1], >src[0], new_instr);
new_instr->src[2] = nir_src_for_ssa(temp);
break;
- case nir_intrinsic_atomic_counter_dec:
+ case nir_intrinsic_atomic_counter_pre_dec:
+ case nir_intrinsic_atomic_counter_pos_dec:
/* remapped to ssbo_atomic_add: { buffer_idx, offset, -1 } */
/* NOTE semantic difference so we adjust the return value below */
temp = nir_imm_int(b, -1);
@@ -148,7 +150,7 @@ lower_instr(nir_intrinsic_instr *instr, unsigned
ssbo_offset, nir_builder *b)
nir_instr_insert_before(>instr, _instr->instr);
nir_instr_remove(>instr);
- if (instr->intrinsic == nir_intrinsic_atomic_counter_dec) {
+
Re: [Mesa-dev] [PATCH 14/18] nir/linker: Add a pure NIR implementation of the atomic counter linker
patches 10-14 are: Reviewed-by: Timothy Arceri ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 07/18] spirv/nir: tweak nir type when storage class is SpvStorageClassAtomicCounter
Patches 4-7: Reviewed-by: Timothy Arceri ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/18] nir/spirv: Fix atomic counter (multidimensional-)arrays
On 30/06/18 00:28, Alejandro Piñeiro wrote:
From: Antia Puentes
When constructing NIR if we have a SPIR-V uint variable and the
storage class is SpvStorageClassAtomicCounter, we store as NIR's
glsl_type an atomic_uint to reflect the fact that the variable is an
atomic counter.
However, we were tweaking the type only for atomic_uint scalars, we
have to do it as well for atomic_uint arrays and atomic_uint arrays of
arrays of any depth.
Signed-off-by: Antia Puentes
Signed-off-by: Alejandro Piñeiro
v2: update after deref patches got pushed (Alejandro Piñeiro)
---
src/compiler/spirv/vtn_variables.c | 40 +++---
1 file changed, 37 insertions(+), 3 deletions(-)
diff --git a/src/compiler/spirv/vtn_variables.c
b/src/compiler/spirv/vtn_variables.c
index a40c30c8a75..c75492ef43f 100644
--- a/src/compiler/spirv/vtn_variables.c
+++ b/src/compiler/spirv/vtn_variables.c
@@ -374,6 +374,41 @@ vtn_pointer_for_variable(struct vtn_builder *b,
return pointer;
}
+/* Returns an atomic_uint type based on the original uint type. The returned
+ * type will be equivalent to the original one but will have an atomic_uint
+ * type as leaf instead of an uint.
+ *
+ * Manages uint scalars, arrays, and arrays of arrays of any nested depth.
+ */
+static const struct glsl_type *
+repair_atomic_type(const struct glsl_type *type, SpvStorageClass storage_class)
+{
+ assert(storage_class == SpvStorageClassAtomicCounter);
+ assert(glsl_get_base_type(glsl_without_array(type)) == GLSL_TYPE_UINT);
+ assert(glsl_type_is_scalar(glsl_without_array(type)));
+
+ const struct glsl_type *atomic = glsl_atomic_uint_type();
+ unsigned depth = glsl_array_depth(type);
+
+ if (depth > 0) {
+ unsigned *lengths = calloc(depth, sizeof(unsigned));
+ unsigned i = depth;
+
+ while (glsl_type_is_array(type)) {
+ i--;
+ lengths[i] = glsl_get_length(type);
+ type = glsl_get_array_element(type);
+ }
+
+ for (i = 0; i < depth; i++)
+ atomic = glsl_array_type(atomic, lengths[i]);
+
+ free(lengths);
+ }
If you write it like this:
static const struct glsl_type *
repair_atomic_type(const struct glsl_type *type)
{
assert(glsl_get_base_type(glsl_without_array(type)) == GLSL_TYPE_UINT);
assert(glsl_type_is_scalar(glsl_without_array(type)));
if (glsl_type_is_array(type)) {
const struct glsl_type *atomic =
repair_atomic_type(glsl_get_array_element(type));
return glsl_array_type(atomic, glsl_get_length(type));
} else {
return glsl_atomic_uint_type();
}
}
We can drop the previous patch and it makes it much easier to follow
IMO. There is no need to pass storage_class to this function. We should
just let the caller check that which it does.
If you agree with the changes. This patch is:
Reviewed-by: Timothy Arceri
+
+ return atomic;
+}
+
nir_deref_instr *
vtn_pointer_to_deref(struct vtn_builder *b, struct vtn_pointer *ptr)
{
@@ -1648,9 +1683,8 @@ vtn_create_variable(struct vtn_builder *b, struct
vtn_value *val,
* the access to storage_class, that is the one that points us that is
* an atomic uint.
*/
- if (glsl_get_base_type(var->type->type) == GLSL_TYPE_UINT &&
- storage_class == SpvStorageClassAtomicCounter) {
- var->var->type = glsl_atomic_uint_type();
+ if (storage_class == SpvStorageClassAtomicCounter) {
+ var->var->type = repair_atomic_type(var->type->type, storage_class);
} else {
var->var->type = var->type->type;
}
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 08/18] compiler: utility to get the depth of multidimensional array
NAK this shouldn't be needed. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/18] nir/linker: use empty block info to assign uniform locations
On 30/06/18 00:28, Alejandro Piñeiro wrote:
For the cases of uniforms that doesn't have an explicit
location. Under ARB_gl_spirv those are exceptions, like uniform atomic
counters.
---
src/compiler/glsl/gl_nir_link_uniforms.c | 31 +--
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/src/compiler/glsl/gl_nir_link_uniforms.c
b/src/compiler/glsl/gl_nir_link_uniforms.c
index 388c1ab63fc..77d3eaa5f2b 100644
--- a/src/compiler/glsl/gl_nir_link_uniforms.c
+++ b/src/compiler/glsl/gl_nir_link_uniforms.c
@@ -79,6 +79,8 @@ nir_setup_uniform_remap_tables(struct gl_context *ctx,
}
/* Reserve locations for rest of the uniforms. */
+ link_util_update_empty_uniform_locations(prog);
+
for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
struct gl_uniform_storage *uniform = >data->UniformStorage[i];
@@ -93,22 +95,23 @@ nir_setup_uniform_remap_tables(struct gl_context *ctx,
if (uniform->remap_location != UNMAPPED_UNIFORM_LOC)
continue;
- /* How many new entries for this uniform? */
+ /* How many entries for this uniform? */
const unsigned entries = MAX2(1, uniform->array_elements);
- /* @FIXME: By now, we add un-assigned uniform locations to the end of
- * the uniform file. We need to keep track of empty locations and use
- * them.
- */
- unsigned chosen_location = prog->NumUniformRemapTable;
-
- /* resize remap table to fit new entries */
- prog->UniformRemapTable =
- reralloc(prog,
- prog->UniformRemapTable,
- struct gl_uniform_storage *,
- prog->NumUniformRemapTable + entries);
- prog->NumUniformRemapTable += entries;
+ unsigned chosen_location =
Please move patch 2 to patch 1 and squash with this one with the
current patch 1. This is just code churn.
As with my review of patch 1 please rename chosen_location -> location
Otherwise:
Reviewed-by: Timothy Arceri
+ link_util_find_empty_block(prog, >data->UniformStorage[i]);
+
+ if (chosen_location == -1) {
+ chosen_location = prog->NumUniformRemapTable;
+
+ /* resize remap table to fit new entries */
+ prog->UniformRemapTable =
+reralloc(prog,
+ prog->UniformRemapTable,
+ struct gl_uniform_storage *,
+ prog->NumUniformRemapTable + entries);
+ prog->NumUniformRemapTable += entries;
+ }
/* set the base location in remap table for the uniform */
uniform->remap_location = chosen_location;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 02/18] compiler/glsl: refactor empty_uniform_block utilities to linker_util
On 30/06/18 00:28, Alejandro Piñeiro wrote:
This includes:
* Move the defition of empty_uniform_block to linker_util.h
* Move find_empty_block (with a rename) to linker_util.h
* Refactor some code at linker.cpp to a new method at linker_util.h
(link_util_update_empty_uniform_locations)
So all that code could be used by the GLSL linker and the NIR linker
used for ARB_gl_spirv.
---
src/compiler/glsl/link_uniforms.cpp | 34 +--
src/compiler/glsl/linker.cpp| 19 ++---
src/compiler/glsl/linker.h | 13 -
src/compiler/glsl/linker_util.cpp | 55 +
src/compiler/glsl/linker_util.h | 21 ++
5 files changed, 79 insertions(+), 63 deletions(-)
diff --git a/src/compiler/glsl/link_uniforms.cpp
b/src/compiler/glsl/link_uniforms.cpp
index 23ff7ec6728..8d3f95fe114 100644
--- a/src/compiler/glsl/link_uniforms.cpp
+++ b/src/compiler/glsl/link_uniforms.cpp
@@ -1153,38 +1153,6 @@ assign_hidden_uniform_slot_id(const char *name, unsigned
hidden_id,
uniform_size->map->put(hidden_uniform_start + hidden_id, name);
}
-/**
- * Search through the list of empty blocks to find one that fits the current
- * uniform.
- */
-static int
-find_empty_block(struct gl_shader_program *prog,
- struct gl_uniform_storage *uniform)
-{
- const unsigned entries = MAX2(1, uniform->array_elements);
-
- foreach_list_typed(struct empty_uniform_block, block, link,
- >EmptyUniformLocations) {
- /* Found a block with enough slots to fit the uniform */
- if (block->slots == entries) {
- unsigned start = block->start;
- exec_node_remove(>link);
- ralloc_free(block);
-
- return start;
- /* Found a block with more slots than needed. It can still be used. */
- } else if (block->slots > entries) {
- unsigned start = block->start;
- block->start += entries;
- block->slots -= entries;
-
- return start;
- }
- }
-
- return -1;
-}
-
static void
link_setup_uniform_remap_tables(struct gl_context *ctx,
struct gl_shader_program *prog)
@@ -1239,7 +1207,7 @@ link_setup_uniform_remap_tables(struct gl_context *ctx,
int chosen_location = -1;
if (empty_locs)
- chosen_location = find_empty_block(prog,
>data->UniformStorage[i]);
+ chosen_location = link_util_find_empty_block(prog,
>data->UniformStorage[i]);
/* Add new entries to the total amount of entries. */
total_entries += entries;
diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
index 95e7c3c5e99..6a9d19e8695 100644
--- a/src/compiler/glsl/linker.cpp
+++ b/src/compiler/glsl/linker.cpp
@@ -76,6 +76,7 @@
#include "util/set.h"
#include "string_to_uint_map.h"
#include "linker.h"
+#include "linker_util.h"
#include "link_varyings.h"
#include "ir_optimization.h"
#include "ir_rvalue_visitor.h"
@@ -3527,23 +3528,7 @@ check_explicit_uniform_locations(struct gl_context *ctx,
}
}
- struct empty_uniform_block *current_block = NULL;
-
- for (unsigned i = 0; i < prog->NumUniformRemapTable; i++) {
- /* We found empty space in UniformRemapTable. */
- if (prog->UniformRemapTable[i] == NULL) {
- /* We've found the beginning of a new continous block of empty slots
*/
- if (!current_block || current_block->start + current_block->slots !=
i) {
-current_block = rzalloc(prog, struct empty_uniform_block);
-current_block->start = i;
-exec_list_push_tail(>EmptyUniformLocations,
-_block->link);
- }
-
- /* The current block continues, so we simply increment its slots */
- current_block->slots++;
- }
- }
+ link_util_update_empty_uniform_locations(prog);
delete uniform_map;
prog->NumExplicitUniformLocations = entries_total;
diff --git a/src/compiler/glsl/linker.h b/src/compiler/glsl/linker.h
index 6e9b4861673..f6fb00351d4 100644
--- a/src/compiler/glsl/linker.h
+++ b/src/compiler/glsl/linker.h
@@ -194,17 +194,4 @@ private:
const glsl_struct_field *named_ifc_member);
};
-/**
- * Sometimes there are empty slots left over in UniformRemapTable after we
- * allocate slots to explicit locations. This struct represents a single
- * continouous block of empty slots in UniformRemapTable.
- */
-struct empty_uniform_block {
- struct exec_node link;
- /* The start location of the block */
- unsigned start;
- /* The number of slots in the block */
- unsigned slots;
-};
-
#endif /* GLSL_LINKER_H */
diff --git a/src/compiler/glsl/linker_util.cpp
b/src/compiler/glsl/linker_util.cpp
index 0e6f4166d64..e80c9e22ae2 100644
--- a/src/compiler/glsl/linker_util.cpp
+++ b/src/compiler/glsl/linker_util.cpp
@@ -24,6 +24,7 @@
#include "main/mtypes.h"
#include "linker_util.h"
Re: [Mesa-dev] [PATCH 01/18] nir/linker: handle uniforms without explicit location
On 30/06/18 16:05, Timothy Arceri wrote:
On 30/06/18 00:28, Alejandro Piñeiro wrote:
ARB_gl_spirv points that uniforms in general need explicit
location. But there are still some cases of uniforms without location,
like for example uniform atomic counters. Those doesn't have a
location from the OpenGL point of view (they are identified with a
binding), but Mesa internally assigns it a location.
Signed-off-by: Eduardo Lima
Signed-off-by: Alejandro Piñeiro
Signed-off-by: Neil Roberts
---
The @FIXME included on the patch below is solved with the follow-up
path "nir/linker: use empty block info to assign uniform locations",
so perhaps it makes sense to just squash both patches. I don't have a
strong opinion on that, but I think that it would be easier to review
as splitted patches.
src/compiler/glsl/gl_nir_link_uniforms.c | 61
++--
1 file changed, 59 insertions(+), 2 deletions(-)
diff --git a/src/compiler/glsl/gl_nir_link_uniforms.c
b/src/compiler/glsl/gl_nir_link_uniforms.c
index c6961fbb6ca..388c1ab63fc 100644
--- a/src/compiler/glsl/gl_nir_link_uniforms.c
+++ b/src/compiler/glsl/gl_nir_link_uniforms.c
@@ -36,6 +36,8 @@
* normal uniforms as mandatory, and so on).
*/
+#define UNMAPPED_UNIFORM_LOC ~0u
+
static void
nir_setup_uniform_remap_tables(struct gl_context *ctx,
struct gl_shader_program *prog)
@@ -58,8 +60,59 @@ nir_setup_uniform_remap_tables(struct gl_context *ctx,
for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
struct gl_uniform_storage *uniform =
>data->UniformStorage[i];
+ if (prog->data->UniformStorage[i].remap_location ==
UNMAPPED_UNIFORM_LOC)
+ continue;
+
+ /* How many new entries for this uniform? */
+ const unsigned entries = MAX2(1, uniform->array_elements);
+ unsigned num_slots = glsl_get_component_slots(uniform->type);
+
+ uniform->storage = [data_pos];
+
+ /* Set remap table entries point to correct gl_uniform_storage. */
+ for (unsigned j = 0; j < entries; j++) {
+ unsigned element_loc = uniform->remap_location + j;
+ prog->UniformRemapTable[element_loc] = uniform;
+
+ data_pos += num_slots;
+ }
+ }
+
+ /* Reserve locations for rest of the uniforms. */
+ for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
+ struct gl_uniform_storage *uniform =
>data->UniformStorage[i];
+
+ if (uniform->is_shader_storage)
+ continue;
+
+ /* Built-in uniforms should not get any location. */
+ if (uniform->builtin)
+ continue;
+
+ /* Explicit ones have been set already. */
+ if (uniform->remap_location != UNMAPPED_UNIFORM_LOC)
+ continue;
+
/* How many new entries for this uniform? */
const unsigned entries = MAX2(1, uniform->array_elements);
+
+ /* @FIXME: By now, we add un-assigned uniform locations to the
end of
+ * the uniform file. We need to keep track of empty locations
and use
+ * them.
+ */
Maybe reword this to:
/* @FIXME: For now, we add un-assigned uniform locations to the end of
* the uniform file. We should fix this to keep track of empty
* locations and use those first.
*/
+ unsigned chosen_location = prog->NumUniformRemapTable;
Can we please just rename chosen_location
Whoops that should be:
Can we please just rename chosen_location -> location
With those two nits this patch is:
Reviewed-by: Timothy Arceri
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/18] nir/linker: handle uniforms without explicit location
On 30/06/18 00:28, Alejandro Piñeiro wrote:
ARB_gl_spirv points that uniforms in general need explicit
location. But there are still some cases of uniforms without location,
like for example uniform atomic counters. Those doesn't have a
location from the OpenGL point of view (they are identified with a
binding), but Mesa internally assigns it a location.
Signed-off-by: Eduardo Lima
Signed-off-by: Alejandro Piñeiro
Signed-off-by: Neil Roberts
---
The @FIXME included on the patch below is solved with the follow-up
path "nir/linker: use empty block info to assign uniform locations",
so perhaps it makes sense to just squash both patches. I don't have a
strong opinion on that, but I think that it would be easier to review
as splitted patches.
src/compiler/glsl/gl_nir_link_uniforms.c | 61 ++--
1 file changed, 59 insertions(+), 2 deletions(-)
diff --git a/src/compiler/glsl/gl_nir_link_uniforms.c
b/src/compiler/glsl/gl_nir_link_uniforms.c
index c6961fbb6ca..388c1ab63fc 100644
--- a/src/compiler/glsl/gl_nir_link_uniforms.c
+++ b/src/compiler/glsl/gl_nir_link_uniforms.c
@@ -36,6 +36,8 @@
* normal uniforms as mandatory, and so on).
*/
+#define UNMAPPED_UNIFORM_LOC ~0u
+
static void
nir_setup_uniform_remap_tables(struct gl_context *ctx,
struct gl_shader_program *prog)
@@ -58,8 +60,59 @@ nir_setup_uniform_remap_tables(struct gl_context *ctx,
for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
struct gl_uniform_storage *uniform = >data->UniformStorage[i];
+ if (prog->data->UniformStorage[i].remap_location == UNMAPPED_UNIFORM_LOC)
+ continue;
+
+ /* How many new entries for this uniform? */
+ const unsigned entries = MAX2(1, uniform->array_elements);
+ unsigned num_slots = glsl_get_component_slots(uniform->type);
+
+ uniform->storage = [data_pos];
+
+ /* Set remap table entries point to correct gl_uniform_storage. */
+ for (unsigned j = 0; j < entries; j++) {
+ unsigned element_loc = uniform->remap_location + j;
+ prog->UniformRemapTable[element_loc] = uniform;
+
+ data_pos += num_slots;
+ }
+ }
+
+ /* Reserve locations for rest of the uniforms. */
+ for (unsigned i = 0; i < prog->data->NumUniformStorage; i++) {
+ struct gl_uniform_storage *uniform = >data->UniformStorage[i];
+
+ if (uniform->is_shader_storage)
+ continue;
+
+ /* Built-in uniforms should not get any location. */
+ if (uniform->builtin)
+ continue;
+
+ /* Explicit ones have been set already. */
+ if (uniform->remap_location != UNMAPPED_UNIFORM_LOC)
+ continue;
+
/* How many new entries for this uniform? */
const unsigned entries = MAX2(1, uniform->array_elements);
+
+ /* @FIXME: By now, we add un-assigned uniform locations to the end of
+ * the uniform file. We need to keep track of empty locations and use
+ * them.
+ */
Maybe reword this to:
/* @FIXME: For now, we add un-assigned uniform locations to the end of
* the uniform file. We should fix this to keep track of empty
* locations and use those first.
*/
+ unsigned chosen_location = prog->NumUniformRemapTable;
Can we please just rename chosen_location
With those two nits this patch is:
Reviewed-by: Timothy Arceri
+
+ /* resize remap table to fit new entries */
+ prog->UniformRemapTable =
+ reralloc(prog,
+ prog->UniformRemapTable,
+ struct gl_uniform_storage *,
+ prog->NumUniformRemapTable + entries);
+ prog->NumUniformRemapTable += entries;
+
+ /* set the base location in remap table for the uniform */
+ uniform->remap_location = chosen_location;
+
unsigned num_slots = glsl_get_component_slots(uniform->type);
uniform->storage = [data_pos];
@@ -302,8 +355,12 @@ nir_link_uniform(struct gl_context *ctx,
}
uniform->active_shader_mask |= 1 << stage;
- /* Uniform has an explicit location */
- uniform->remap_location = location;
+ if (location >= 0) {
+ /* Uniform has an explicit location */
+ uniform->remap_location = location;
+ } else {
+ uniform->remap_location = UNMAPPED_UNIFORM_LOC;
+ }
/* @FIXME: the initialization of the following will be done as we
* implement support for their specific features, like SSBO, atomics,
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
