Discussion:
[PATCH 1/3] amd/common: scan if gl_InvocationID is used
Add Reply
Samuel Pitoiset
2017-12-20 19:56:55 UTC
Reply
Permalink
Raw Message
Signed-off-by: Samuel Pitoiset <***@gmail.com>
---
src/amd/common/ac_shader_info.c | 3 +++
src/amd/common/ac_shader_info.h | 1 +
2 files changed, 4 insertions(+)

diff --git a/src/amd/common/ac_shader_info.c b/src/amd/common/ac_shader_info.c
index ab5388fb53..5dac1131bd 100644
--- a/src/amd/common/ac_shader_info.c
+++ b/src/amd/common/ac_shader_info.c
@@ -70,6 +70,9 @@ gather_intrinsic_info(nir_intrinsic_instr *instr, struct ac_shader_info *info)
case nir_intrinsic_load_view_index:
info->needs_multiview_view_index = true;
break;
+ case nir_intrinsic_load_invocation_id:
+ info->uses_invocation_id = true;
+ break;
case nir_intrinsic_vulkan_resource_index:
info->desc_set_used_mask |= (1 << nir_intrinsic_desc_set(instr));
break;
diff --git a/src/amd/common/ac_shader_info.h b/src/amd/common/ac_shader_info.h
index 79e5615254..7c79d1a728 100644
--- a/src/amd/common/ac_shader_info.h
+++ b/src/amd/common/ac_shader_info.h
@@ -31,6 +31,7 @@ struct ac_shader_info {
bool needs_push_constants;
uint32_t desc_set_used_mask;
bool needs_multiview_view_index;
+ bool uses_invocation_id;
struct {
bool has_vertex_buffers; /* needs vertex buffers and base/start */
bool needs_draw_id;
--
2.15.1
Samuel Pitoiset
2017-12-20 19:56:57 UTC
Reply
Permalink
Raw Message
This can still be improved, but let's start with this.

Signed-off-by: Samuel Pitoiset <***@gmail.com>
---
src/amd/vulkan/radv_shader.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/amd/vulkan/radv_shader.c b/src/amd/vulkan/radv_shader.c
index ab8ba42511..31879805ae 100644
--- a/src/amd/vulkan/radv_shader.c
+++ b/src/amd/vulkan/radv_shader.c
@@ -411,8 +411,21 @@ radv_fill_shader_variant(struct radv_device *device,

if (device->physical_device->rad_info.chip_class >= GFX9 &&
stage == MESA_SHADER_GEOMETRY) {
+ struct ac_shader_info *info = &variant->info.info;
+ unsigned gs_vgpr_comp_cnt;
+
+ /* If offsets 4, 5 are used, GS_VGPR_COMP_CNT is ignored and
+ * VGPR[0:4] are always loaded.
+ */
+ if (info->uses_invocation_id)
+ gs_vgpr_comp_cnt = 3; /* VGPR3 contains InvocationID. */
+ else if (info->uses_prim_id)
+ gs_vgpr_comp_cnt = 2; /* VGPR2 contains PrimitiveID. */
+ else
+ gs_vgpr_comp_cnt = 1; /* TODO: use input_prim */
+
/* TODO: Figure out how many we actually need. */
- variant->rsrc1 |= S_00B228_GS_VGPR_COMP_CNT(3);
+ variant->rsrc1 |= S_00B228_GS_VGPR_COMP_CNT(gs_vgpr_comp_cnt);
variant->rsrc2 |= S_00B22C_ES_VGPR_COMP_CNT(3) |
S_00B22C_OC_LDS_EN(1);
} else if (device->physical_device->rad_info.chip_class >= GFX9 &&
--
2.15.1
Samuel Pitoiset
2017-12-20 19:56:56 UTC
Reply
Permalink
Raw Message
It makes more sense to move all scan stuff in the same place.
Also, we don't really need to duplicate the uses_primid field
for each stages.

Signed-off-by: Samuel Pitoiset <***@gmail.com>
---
src/amd/common/ac_nir_to_llvm.c | 4 ----
src/amd/common/ac_nir_to_llvm.h | 3 ---
src/amd/common/ac_shader_info.c | 3 +++
src/amd/common/ac_shader_info.h | 1 +
src/amd/vulkan/radv_pipeline.c | 6 +++---
5 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index dcd99a5956..788e91aeb8 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -4208,14 +4208,10 @@ static void visit_intrinsic(struct ac_nir_context *ctx,
break;
case nir_intrinsic_load_primitive_id:
if (ctx->stage == MESA_SHADER_GEOMETRY) {
- if (ctx->nctx)
- ctx->nctx->shader_info->gs.uses_prim_id = true;
result = ctx->abi->gs_prim_id;
} else if (ctx->stage == MESA_SHADER_TESS_CTRL) {
- ctx->nctx->shader_info->tcs.uses_prim_id = true;
result = ctx->nctx->tcs_patch_id;
} else if (ctx->stage == MESA_SHADER_TESS_EVAL) {
- ctx->nctx->shader_info->tcs.uses_prim_id = true;
result = ctx->nctx->tes_patch_id;
} else
fprintf(stderr, "Unknown primitive id intrinsic: %d", ctx->stage);
diff --git a/src/amd/common/ac_nir_to_llvm.h b/src/amd/common/ac_nir_to_llvm.h
index 1d9ec8ce8b..6c59ab916c 100644
--- a/src/amd/common/ac_nir_to_llvm.h
+++ b/src/amd/common/ac_nir_to_llvm.h
@@ -191,10 +191,8 @@ struct ac_shader_variant_info {
unsigned invocations;
unsigned gsvs_vertex_size;
unsigned max_gsvs_emit_size;
- bool uses_prim_id;
} gs;
struct {
- bool uses_prim_id;
unsigned tcs_vertices_out;
/* Which outputs are actually written */
uint64_t outputs_written;
@@ -210,7 +208,6 @@ struct ac_shader_variant_info {
enum gl_tess_spacing spacing;
bool ccw;
bool point_mode;
- bool uses_prim_id;
} tes;
};
};
diff --git a/src/amd/common/ac_shader_info.c b/src/amd/common/ac_shader_info.c
index 5dac1131bd..27896a26bb 100644
--- a/src/amd/common/ac_shader_info.c
+++ b/src/amd/common/ac_shader_info.c
@@ -73,6 +73,9 @@ gather_intrinsic_info(nir_intrinsic_instr *instr, struct ac_shader_info *info)
case nir_intrinsic_load_invocation_id:
info->uses_invocation_id = true;
break;
+ case nir_intrinsic_load_primitive_id:
+ info->uses_prim_id = true;
+ break;
case nir_intrinsic_vulkan_resource_index:
info->desc_set_used_mask |= (1 << nir_intrinsic_desc_set(instr));
break;
diff --git a/src/amd/common/ac_shader_info.h b/src/amd/common/ac_shader_info.h
index 7c79d1a728..437859f891 100644
--- a/src/amd/common/ac_shader_info.h
+++ b/src/amd/common/ac_shader_info.h
@@ -32,6 +32,7 @@ struct ac_shader_info {
uint32_t desc_set_used_mask;
bool needs_multiview_view_index;
bool uses_invocation_id;
+ bool uses_prim_id;
struct {
bool has_vertex_buffers; /* needs vertex buffers and base/start */
bool needs_draw_id;
diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index 3fc21bb501..d725f9b891 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -2191,12 +2191,12 @@ radv_pipeline_init(struct radv_pipeline *pipeline,
if (pipeline->shaders[MESA_SHADER_FRAGMENT]->info.fs.prim_id_input)
pipeline->graphics.ia_switch_on_eoi = true;
if (radv_pipeline_has_gs(pipeline) &&
- pipeline->shaders[MESA_SHADER_GEOMETRY]->info.gs.uses_prim_id)
+ pipeline->shaders[MESA_SHADER_GEOMETRY]->info.info.uses_prim_id)
pipeline->graphics.ia_switch_on_eoi = true;
if (radv_pipeline_has_tess(pipeline)) {
/* SWITCH_ON_EOI must be set if PrimID is used. */
- if (pipeline->shaders[MESA_SHADER_TESS_CTRL]->info.tcs.uses_prim_id ||
- radv_get_tess_eval_shader(pipeline)->info.tes.uses_prim_id)
+ if (pipeline->shaders[MESA_SHADER_TESS_CTRL]->info.info.uses_prim_id ||
+ radv_get_tess_eval_shader(pipeline)->info.info.uses_prim_id)
pipeline->graphics.ia_switch_on_eoi = true;
}
--
2.15.1
Marek Olšák
2017-12-26 17:41:03 UTC
Reply
Permalink
Raw Message
Reviewed-by: Marek Olšák <***@amd.com>

Marek

On Wed, Dec 20, 2017 at 8:56 PM, Samuel Pitoiset
Post by Samuel Pitoiset
It makes more sense to move all scan stuff in the same place.
Also, we don't really need to duplicate the uses_primid field
for each stages.
---
src/amd/common/ac_nir_to_llvm.c | 4 ----
src/amd/common/ac_nir_to_llvm.h | 3 ---
src/amd/common/ac_shader_info.c | 3 +++
src/amd/common/ac_shader_info.h | 1 +
src/amd/vulkan/radv_pipeline.c | 6 +++---
5 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c
index dcd99a5956..788e91aeb8 100644
--- a/src/amd/common/ac_nir_to_llvm.c
+++ b/src/amd/common/ac_nir_to_llvm.c
@@ -4208,14 +4208,10 @@ static void visit_intrinsic(struct ac_nir_context *ctx,
break;
if (ctx->stage == MESA_SHADER_GEOMETRY) {
- if (ctx->nctx)
- ctx->nctx->shader_info->gs.uses_prim_id = true;
result = ctx->abi->gs_prim_id;
} else if (ctx->stage == MESA_SHADER_TESS_CTRL) {
- ctx->nctx->shader_info->tcs.uses_prim_id = true;
result = ctx->nctx->tcs_patch_id;
} else if (ctx->stage == MESA_SHADER_TESS_EVAL) {
- ctx->nctx->shader_info->tcs.uses_prim_id = true;
result = ctx->nctx->tes_patch_id;
} else
fprintf(stderr, "Unknown primitive id intrinsic: %d", ctx->stage);
diff --git a/src/amd/common/ac_nir_to_llvm.h b/src/amd/common/ac_nir_to_llvm.h
index 1d9ec8ce8b..6c59ab916c 100644
--- a/src/amd/common/ac_nir_to_llvm.h
+++ b/src/amd/common/ac_nir_to_llvm.h
@@ -191,10 +191,8 @@ struct ac_shader_variant_info {
unsigned invocations;
unsigned gsvs_vertex_size;
unsigned max_gsvs_emit_size;
- bool uses_prim_id;
} gs;
struct {
- bool uses_prim_id;
unsigned tcs_vertices_out;
/* Which outputs are actually written */
uint64_t outputs_written;
@@ -210,7 +208,6 @@ struct ac_shader_variant_info {
enum gl_tess_spacing spacing;
bool ccw;
bool point_mode;
- bool uses_prim_id;
} tes;
};
};
diff --git a/src/amd/common/ac_shader_info.c b/src/amd/common/ac_shader_info.c
index 5dac1131bd..27896a26bb 100644
--- a/src/amd/common/ac_shader_info.c
+++ b/src/amd/common/ac_shader_info.c
@@ -73,6 +73,9 @@ gather_intrinsic_info(nir_intrinsic_instr *instr, struct ac_shader_info *info)
info->uses_invocation_id = true;
break;
+ info->uses_prim_id = true;
+ break;
info->desc_set_used_mask |= (1 << nir_intrinsic_desc_set(instr));
break;
diff --git a/src/amd/common/ac_shader_info.h b/src/amd/common/ac_shader_info.h
index 7c79d1a728..437859f891 100644
--- a/src/amd/common/ac_shader_info.h
+++ b/src/amd/common/ac_shader_info.h
@@ -32,6 +32,7 @@ struct ac_shader_info {
uint32_t desc_set_used_mask;
bool needs_multiview_view_index;
bool uses_invocation_id;
+ bool uses_prim_id;
struct {
bool has_vertex_buffers; /* needs vertex buffers and base/start */
bool needs_draw_id;
diff --git a/src/amd/vulkan/radv_pipeline.c b/src/amd/vulkan/radv_pipeline.c
index 3fc21bb501..d725f9b891 100644
--- a/src/amd/vulkan/radv_pipeline.c
+++ b/src/amd/vulkan/radv_pipeline.c
@@ -2191,12 +2191,12 @@ radv_pipeline_init(struct radv_pipeline *pipeline,
if (pipeline->shaders[MESA_SHADER_FRAGMENT]->info.fs.prim_id_input)
pipeline->graphics.ia_switch_on_eoi = true;
if (radv_pipeline_has_gs(pipeline) &&
- pipeline->shaders[MESA_SHADER_GEOMETRY]->info.gs.uses_prim_id)
+ pipeline->shaders[MESA_SHADER_GEOMETRY]->info.info.uses_prim_id)
pipeline->graphics.ia_switch_on_eoi = true;
if (radv_pipeline_has_tess(pipeline)) {
/* SWITCH_ON_EOI must be set if PrimID is used. */
- if (pipeline->shaders[MESA_SHADER_TESS_CTRL]->info.tcs.uses_prim_id ||
- radv_get_tess_eval_shader(pipeline)->info.tes.uses_prim_id)
+ if (pipeline->shaders[MESA_SHADER_TESS_CTRL]->info.info.uses_prim_id ||
+ radv_get_tess_eval_shader(pipeline)->info.info.uses_prim_id)
pipeline->graphics.ia_switch_on_eoi = true;
}
--
2.15.1
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Marek Olšák
2017-12-26 17:40:06 UTC
Reply
Permalink
Raw Message
Reviewed-by: Marek Olšák <***@amd.com>

Marek

On Wed, Dec 20, 2017 at 8:56 PM, Samuel Pitoiset
Post by Samuel Pitoiset
---
src/amd/common/ac_shader_info.c | 3 +++
src/amd/common/ac_shader_info.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/src/amd/common/ac_shader_info.c b/src/amd/common/ac_shader_info.c
index ab5388fb53..5dac1131bd 100644
--- a/src/amd/common/ac_shader_info.c
+++ b/src/amd/common/ac_shader_info.c
@@ -70,6 +70,9 @@ gather_intrinsic_info(nir_intrinsic_instr *instr, struct ac_shader_info *info)
info->needs_multiview_view_index = true;
break;
+ info->uses_invocation_id = true;
+ break;
info->desc_set_used_mask |= (1 << nir_intrinsic_desc_set(instr));
break;
diff --git a/src/amd/common/ac_shader_info.h b/src/amd/common/ac_shader_info.h
index 79e5615254..7c79d1a728 100644
--- a/src/amd/common/ac_shader_info.h
+++ b/src/amd/common/ac_shader_info.h
@@ -31,6 +31,7 @@ struct ac_shader_info {
bool needs_push_constants;
uint32_t desc_set_used_mask;
bool needs_multiview_view_index;
+ bool uses_invocation_id;
struct {
bool has_vertex_buffers; /* needs vertex buffers and base/start */
bool needs_draw_id;
--
2.15.1
_______________________________________________
mesa-dev mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Loading...