Re: [Mesa-dev] [PATCH 01/19] radeonsi: clean up passing the is_monolithic flag for compilation
Hello Marek, after this series landed I get this: Making all in targets/pipe-loader make[4]: Verzeichnis „/opt/mesa/src/gallium/targets/pipe-loader“ wird betreten CXXLDpipe_r600.la ../../../../src/gallium/winsys/radeon/drm/.libs/libradeonwinsys.a(radeon_drm_surface.o): In function `radeon_winsys_surface_init': /opt/mesa/src/gallium/winsys/radeon/drm/radeon_drm_surface.c:307: undefined reference to `ac_compute_cmask' collect2: error: ld returned 1 exit status make[4]: *** [Makefile:970: pipe_r600.la] Fehler 1 Didn't have more time for digging, yet. Dieter Am 23.06.2018 00:31, schrieb Marek Olšák: From: Marek Olšák --- src/gallium/drivers/radeonsi/si_shader.c | 30 +-- src/gallium/drivers/radeonsi/si_shader.h | 1 - .../drivers/radeonsi/si_shader_internal.h | 3 -- .../drivers/radeonsi/si_state_shaders.c | 7 +++-- 4 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c index e7e2a12a7b0..677853af60b 100644 --- a/src/gallium/drivers/radeonsi/si_shader.c +++ b/src/gallium/drivers/radeonsi/si_shader.c @@ -5047,22 +5047,21 @@ static void create_function(struct si_shader_context *ctx) break; default: assert(0 && "unimplemented shader"); return; } si_create_function(ctx, "main", returns, num_returns, &fninfo, si_get_max_workgroup_size(shader)); /* Reserve register locations for VGPR inputs the PS prolog may need. */ - if (ctx->type == PIPE_SHADER_FRAGMENT && - ctx->separate_prolog) { + if (ctx->type == PIPE_SHADER_FRAGMENT && !ctx->shader->is_monolithic) { ac_llvm_add_target_dep_function_attr(ctx->main_fn, "InitialPSInputAddr", S_0286D0_PERSP_SAMPLE_ENA(1) | S_0286D0_PERSP_CENTER_ENA(1) | S_0286D0_PERSP_CENTROID_ENA(1) | S_0286D0_LINEAR_SAMPLE_ENA(1) | S_0286D0_LINEAR_CENTER_ENA(1) | S_0286D0_LINEAR_CENTROID_ENA(1) | S_0286D0_FRONT_FACE_ENA(1) | S_0286D0_ANCILLARY_ENA(1) | @@ -6049,22 +6048,21 @@ static void si_init_exec_from_input(struct si_shader_context *ctx, } static bool si_vs_needs_prolog(const struct si_shader_selector *sel, const struct si_vs_prolog_bits *key) { /* VGPR initialization fixup for Vega10 and Raven is always done in the * VS prolog. */ return sel->vs_needs_prolog || key->ls_vgpr_fix; } -static bool si_compile_tgsi_main(struct si_shader_context *ctx, -bool is_monolithic) +static bool si_compile_tgsi_main(struct si_shader_context *ctx) { struct si_shader *shader = ctx->shader; struct si_shader_selector *sel = shader->selector; struct lp_build_tgsi_context *bld_base = &ctx->bld_base; // TODO clean all this up! switch (ctx->type) { case PIPE_SHADER_VERTEX: ctx->load_input = declare_input_vs; if (shader->key.as_ls) @@ -6135,31 +6133,31 @@ static bool si_compile_tgsi_main(struct si_shader_context *ctx, * - Add a barrier before the second shader. * - In the second shader, reset EXEC to ~0 and wrap the main part in * an if-statement. This is required for correctness in geometry * shaders, to ensure that empty GS waves do not send GS_EMIT and * GS_CUT messages. * * For monolithic merged shaders, the first shader is wrapped in an * if-block together with its prolog in si_build_wrapper_function. */ if (ctx->screen->info.chip_class >= GFX9) { - if (!is_monolithic && + if (!shader->is_monolithic && sel->info.num_instructions > 1 && /* not empty shader */ (shader->key.as_es || shader->key.as_ls) && (ctx->type == PIPE_SHADER_TESS_EVAL || (ctx->type == PIPE_SHADER_VERTEX && !si_vs_needs_prolog(sel, &shader->key.part.vs.prolog { si_init_exec_from_input(ctx, ctx->param_merged_wave_info, 0); } else if (ctx->type == PIPE_SHADER_TESS_CTRL || ctx->type == PIPE_SHADER_GEOMETRY) { - if (!is_monolithic) + if (!shader->is_monolithic) ac_init_exec_full_mask(&ctx->ac);
Re: [Mesa-dev] [PATCH 01/19] radeonsi: clean up passing the is_monolithic flag for compilation
If I disable 'r600' all is fine, but... --with-gallium-drivers=radeonsi,swrast --with-gallium-drivers=r600,radeonsi,swrast Worked before OK. Dieter Am 26.06.2018 01:51, schrieb Dieter Nützel: Hello Marek, after this series landed I get this: Making all in targets/pipe-loader make[4]: Verzeichnis „/opt/mesa/src/gallium/targets/pipe-loader“ wird betreten CXXLDpipe_r600.la ../../../../src/gallium/winsys/radeon/drm/.libs/libradeonwinsys.a(radeon_drm_surface.o): In function `radeon_winsys_surface_init': /opt/mesa/src/gallium/winsys/radeon/drm/radeon_drm_surface.c:307: undefined reference to `ac_compute_cmask' collect2: error: ld returned 1 exit status make[4]: *** [Makefile:970: pipe_r600.la] Fehler 1 Didn't have more time for digging, yet. Dieter Am 23.06.2018 00:31, schrieb Marek Olšák: From: Marek Olšák --- src/gallium/drivers/radeonsi/si_shader.c | 30 +-- src/gallium/drivers/radeonsi/si_shader.h | 1 - .../drivers/radeonsi/si_shader_internal.h | 3 -- .../drivers/radeonsi/si_state_shaders.c | 7 +++-- 4 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c index e7e2a12a7b0..677853af60b 100644 --- a/src/gallium/drivers/radeonsi/si_shader.c +++ b/src/gallium/drivers/radeonsi/si_shader.c @@ -5047,22 +5047,21 @@ static void create_function(struct si_shader_context *ctx) break; default: assert(0 && "unimplemented shader"); return; } si_create_function(ctx, "main", returns, num_returns, &fninfo, si_get_max_workgroup_size(shader)); /* Reserve register locations for VGPR inputs the PS prolog may need. */ - if (ctx->type == PIPE_SHADER_FRAGMENT && - ctx->separate_prolog) { + if (ctx->type == PIPE_SHADER_FRAGMENT && !ctx->shader->is_monolithic) { ac_llvm_add_target_dep_function_attr(ctx->main_fn, "InitialPSInputAddr", S_0286D0_PERSP_SAMPLE_ENA(1) | S_0286D0_PERSP_CENTER_ENA(1) | S_0286D0_PERSP_CENTROID_ENA(1) | S_0286D0_LINEAR_SAMPLE_ENA(1) | S_0286D0_LINEAR_CENTER_ENA(1) | S_0286D0_LINEAR_CENTROID_ENA(1) | S_0286D0_FRONT_FACE_ENA(1) | S_0286D0_ANCILLARY_ENA(1) | @@ -6049,22 +6048,21 @@ static void si_init_exec_from_input(struct si_shader_context *ctx, } static bool si_vs_needs_prolog(const struct si_shader_selector *sel, const struct si_vs_prolog_bits *key) { /* VGPR initialization fixup for Vega10 and Raven is always done in the * VS prolog. */ return sel->vs_needs_prolog || key->ls_vgpr_fix; } -static bool si_compile_tgsi_main(struct si_shader_context *ctx, -bool is_monolithic) +static bool si_compile_tgsi_main(struct si_shader_context *ctx) { struct si_shader *shader = ctx->shader; struct si_shader_selector *sel = shader->selector; struct lp_build_tgsi_context *bld_base = &ctx->bld_base; // TODO clean all this up! switch (ctx->type) { case PIPE_SHADER_VERTEX: ctx->load_input = declare_input_vs; if (shader->key.as_ls) @@ -6135,31 +6133,31 @@ static bool si_compile_tgsi_main(struct si_shader_context *ctx, * - Add a barrier before the second shader. * - In the second shader, reset EXEC to ~0 and wrap the main part in * an if-statement. This is required for correctness in geometry * shaders, to ensure that empty GS waves do not send GS_EMIT and * GS_CUT messages. * * For monolithic merged shaders, the first shader is wrapped in an * if-block together with its prolog in si_build_wrapper_function. */ if (ctx->screen->info.chip_class >= GFX9) { - if (!is_monolithic && + if (!shader->is_monolithic && sel->info.num_instructions > 1 && /* not empty shader */ (shader->key.as_es || shader->key.as_ls) && (ctx->type == PIPE_SHADER_TESS_EVAL || (ctx->type == PIPE_SHADER_VERTEX && !si_vs_needs_prolog(sel, &shader->key.part.vs.prolog { si_init_exec_from_input(ctx, ctx->param_merged_wave_info, 0); } else if (ctx->type == PIPE_SHADER_TESS_CTRL || ctx->type == PIP
Re: [Mesa-dev] [PATCH 01/19] radeonsi: clean up passing the is_monolithic flag for compilation
On 26/06/18 09:51, Dieter Nützel wrote: Hello Marek, after this series landed I get this: Making all in targets/pipe-loader make[4]: Verzeichnis „/opt/mesa/src/gallium/targets/pipe-loader“ wird betreten CXXLD pipe_r600.la ../../../../src/gallium/winsys/radeon/drm/.libs/libradeonwinsys.a(radeon_drm_surface.o): In function `radeon_winsys_surface_init': /opt/mesa/src/gallium/winsys/radeon/drm/radeon_drm_surface.c:307: undefined reference to `ac_compute_cmask' collect2: error: ld returned 1 exit status make[4]: *** [Makefile:970: pipe_r600.la] Fehler 1 Didn't have more time for digging, yet. r600 probably doesn't get linked to the amd common (ac) code that is normally just shared between radv and radeonsi. Dieter Am 23.06.2018 00:31, schrieb Marek Olšák: From: Marek Olšák --- src/gallium/drivers/radeonsi/si_shader.c | 30 +-- src/gallium/drivers/radeonsi/si_shader.h | 1 - .../drivers/radeonsi/si_shader_internal.h | 3 -- .../drivers/radeonsi/si_state_shaders.c | 7 +++-- 4 files changed, 18 insertions(+), 23 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c index e7e2a12a7b0..677853af60b 100644 --- a/src/gallium/drivers/radeonsi/si_shader.c +++ b/src/gallium/drivers/radeonsi/si_shader.c @@ -5047,22 +5047,21 @@ static void create_function(struct si_shader_context *ctx) break; default: assert(0 && "unimplemented shader"); return; } si_create_function(ctx, "main", returns, num_returns, &fninfo, si_get_max_workgroup_size(shader)); /* Reserve register locations for VGPR inputs the PS prolog may need. */ - if (ctx->type == PIPE_SHADER_FRAGMENT && - ctx->separate_prolog) { + if (ctx->type == PIPE_SHADER_FRAGMENT && !ctx->shader->is_monolithic) { ac_llvm_add_target_dep_function_attr(ctx->main_fn, "InitialPSInputAddr", S_0286D0_PERSP_SAMPLE_ENA(1) | S_0286D0_PERSP_CENTER_ENA(1) | S_0286D0_PERSP_CENTROID_ENA(1) | S_0286D0_LINEAR_SAMPLE_ENA(1) | S_0286D0_LINEAR_CENTER_ENA(1) | S_0286D0_LINEAR_CENTROID_ENA(1) | S_0286D0_FRONT_FACE_ENA(1) | S_0286D0_ANCILLARY_ENA(1) | @@ -6049,22 +6048,21 @@ static void si_init_exec_from_input(struct si_shader_context *ctx, } static bool si_vs_needs_prolog(const struct si_shader_selector *sel, const struct si_vs_prolog_bits *key) { /* VGPR initialization fixup for Vega10 and Raven is always done in the * VS prolog. */ return sel->vs_needs_prolog || key->ls_vgpr_fix; } -static bool si_compile_tgsi_main(struct si_shader_context *ctx, - bool is_monolithic) +static bool si_compile_tgsi_main(struct si_shader_context *ctx) { struct si_shader *shader = ctx->shader; struct si_shader_selector *sel = shader->selector; struct lp_build_tgsi_context *bld_base = &ctx->bld_base; // TODO clean all this up! switch (ctx->type) { case PIPE_SHADER_VERTEX: ctx->load_input = declare_input_vs; if (shader->key.as_ls) @@ -6135,31 +6133,31 @@ static bool si_compile_tgsi_main(struct si_shader_context *ctx, * - Add a barrier before the second shader. * - In the second shader, reset EXEC to ~0 and wrap the main part in * an if-statement. This is required for correctness in geometry * shaders, to ensure that empty GS waves do not send GS_EMIT and * GS_CUT messages. * * For monolithic merged shaders, the first shader is wrapped in an * if-block together with its prolog in si_build_wrapper_function. */ if (ctx->screen->info.chip_class >= GFX9) { - if (!is_monolithic && + if (!shader->is_monolithic && sel->info.num_instructions > 1 && /* not empty shader */ (shader->key.as_es || shader->key.as_ls) && (ctx->type == PIPE_SHADER_TESS_EVAL || (ctx->type == PIPE_SHADER_VERTEX && !si_vs_needs_prolog(sel, &shader->key.part.vs.prolog { si_init_exec_from_input(ctx, ctx->param_merged_wave_info, 0); } else if (ctx->type == PIPE_SHADER_TESS_CTRL || ctx->type == PIPE_SHADER_GEOMETRY) { - if (!is_monolithic) + if (!shader->is_monolithic) ac_init_exec_full_mask(&ctx->ac); LLVMValueRef num_threads = si_unpack_param(ctx, ctx->param_merged_wave_info, 8, 8); LLVMValueRef ena = LLVMBuildICmp(ctx->ac.builder, LLVMIntULT, ac_get_thread_id(&ctx->ac), num_threads, ""); lp_build_if(&ctx->merged_wrap_if_state, &ctx->gallivm, ena
Re: [Mesa-dev] [PATCH 01/19] radeonsi: clean up passing the is_monolithic flag for compilation
R300 had the same problem. Linking to amd_common ends up pulling in addrlib too. Jfyi On June 25, 2018 5:48:51 PM PDT, Timothy Arceri wrote: >On 26/06/18 09:51, Dieter Nützel wrote: >> Hello Marek, >> >> after this series landed I get this: >> >> Making all in targets/pipe-loader >> make[4]: Verzeichnis „/opt/mesa/src/gallium/targets/pipe-loader“ wird > >> betreten >> CXXLD pipe_r600.la >> >../../../../src/gallium/winsys/radeon/drm/.libs/libradeonwinsys.a(radeon_drm_surface.o): > >> In function `radeon_winsys_surface_init': >> /opt/mesa/src/gallium/winsys/radeon/drm/radeon_drm_surface.c:307: >> undefined reference to `ac_compute_cmask' >> collect2: error: ld returned 1 exit status >> make[4]: *** [Makefile:970: pipe_r600.la] Fehler 1 >> >> Didn't have more time for digging, yet. > >r600 probably doesn't get linked to the amd common (ac) code that is >normally just shared between radv and radeonsi. > >> >> Dieter >> >> Am 23.06.2018 00:31, schrieb Marek Olšák: >>> From: Marek Olšák >>> >>> --- >>> src/gallium/drivers/radeonsi/si_shader.c | 30 >+-- >>> src/gallium/drivers/radeonsi/si_shader.h | 1 - >>> .../drivers/radeonsi/si_shader_internal.h | 3 -- >>> .../drivers/radeonsi/si_state_shaders.c | 7 +++-- >>> 4 files changed, 18 insertions(+), 23 deletions(-) >>> >>> diff --git a/src/gallium/drivers/radeonsi/si_shader.c >>> b/src/gallium/drivers/radeonsi/si_shader.c >>> index e7e2a12a7b0..677853af60b 100644 >>> --- a/src/gallium/drivers/radeonsi/si_shader.c >>> +++ b/src/gallium/drivers/radeonsi/si_shader.c >>> @@ -5047,22 +5047,21 @@ static void create_function(struct >>> si_shader_context *ctx) >>> break; >>> default: >>> assert(0 && "unimplemented shader"); >>> return; >>> } >>> >>> si_create_function(ctx, "main", returns, num_returns, &fninfo, >>> si_get_max_workgroup_size(shader)); >>> >>> /* Reserve register locations for VGPR inputs the PS prolog may > >>> need. */ >>> - if (ctx->type == PIPE_SHADER_FRAGMENT && >>> - ctx->separate_prolog) { >>> + if (ctx->type == PIPE_SHADER_FRAGMENT && >>> !ctx->shader->is_monolithic) { >>> ac_llvm_add_target_dep_function_attr(ctx->main_fn, >>> "InitialPSInputAddr", >>> S_0286D0_PERSP_SAMPLE_ENA(1) | >>> S_0286D0_PERSP_CENTER_ENA(1) | >>> S_0286D0_PERSP_CENTROID_ENA(1) | >>> S_0286D0_LINEAR_SAMPLE_ENA(1) | >>> S_0286D0_LINEAR_CENTER_ENA(1) | >>> S_0286D0_LINEAR_CENTROID_ENA(1) | >>> S_0286D0_FRONT_FACE_ENA(1) | >>> S_0286D0_ANCILLARY_ENA(1) | >>> @@ -6049,22 +6048,21 @@ static void si_init_exec_from_input(struct >>> si_shader_context *ctx, >>> } >>> >>> static bool si_vs_needs_prolog(const struct si_shader_selector >*sel, >>> const struct si_vs_prolog_bits *key) >>> { >>> /* VGPR initialization fixup for Vega10 and Raven is always >done >>> in the >>> * VS prolog. */ >>> return sel->vs_needs_prolog || key->ls_vgpr_fix; >>> } >>> >>> -static bool si_compile_tgsi_main(struct si_shader_context *ctx, >>> - bool is_monolithic) >>> +static bool si_compile_tgsi_main(struct si_shader_context *ctx) >>> { >>> struct si_shader *shader = ctx->shader; >>> struct si_shader_selector *sel = shader->selector; >>> struct lp_build_tgsi_context *bld_base = &ctx->bld_base; >>> >>> // TODO clean all this up! >>> switch (ctx->type) { >>> case PIPE_SHADER_VERTEX: >>> ctx->load_input = declare_input_vs; >>> if (shader->key.as_ls) >>> @@ -6135,31 +6133,31 @@ static bool si_compile_tgsi_main(struct >>> si_shader_context *ctx, >>> * - Add a barrier before the second shader. >>> * - In the second shader, reset EXEC to ~0 and wrap the main >>> part in >>> * an if-statement. This is required for correctness in >geometry >>> * shaders, to ensure that empty GS waves do not send GS_EMIT >and >>> * GS_CUT messages. >>> * >>> * For monolithic merged shaders, the first shader is wrapped >in an >>> * if-block together with its prolog in >si_build_wrapper_function. >>> */ >>> if (ctx->screen->info.chip_class >= GFX9) { >>> - if (!is_monolithic && >>> + if (!shader->is_monolithic && >>> sel->info.num_instructions > 1 && /* not empty shader >*/ >>> (shader->key.as_es || shader->key.as_ls) && >>> (ctx->type == PIPE_SHADER_TESS_EVAL || >>> (ctx->type == PIPE_SHADER_VERTEX && >>> !si_vs_needs_prolog(sel, >&shader->key.part.vs.prolog { >>> si_init_exec_from_input(ctx, >>> ctx->param_merged_wave_info, 0); >>>
Re: [Mesa-dev] [PATCH 01/19] radeonsi: clean up passing the is_monolithic flag for compilation
On 26 June 2018 at 01:48, Timothy Arceri wrote: > On 26/06/18 09:51, Dieter Nützel wrote: >> >> Hello Marek, >> >> after this series landed I get this: >> >> Making all in targets/pipe-loader >> make[4]: Verzeichnis „/opt/mesa/src/gallium/targets/pipe-loader“ wird >> betreten >>CXXLDpipe_r600.la >> >> ../../../../src/gallium/winsys/radeon/drm/.libs/libradeonwinsys.a(radeon_drm_surface.o): >> In function `radeon_winsys_surface_init': >> /opt/mesa/src/gallium/winsys/radeon/drm/radeon_drm_surface.c:307: >> undefined reference to `ac_compute_cmask' >> collect2: error: ld returned 1 exit status >> make[4]: *** [Makefile:970: pipe_r600.la] Fehler 1 >> >> Didn't have more time for digging, yet. > > > r600 probably doesn't get linked to the amd common (ac) code that is > normally just shared between radv and radeonsi. > Precisely. A quick and simple solution would be to duplicate the function in winsys/radeon. Since neither R300 nor R600 require AC [or the addrlib that gets pulled], this might be the less invasive workaround. What do you guys think? -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 01/19] radeonsi: clean up passing the is_monolithic flag for compilation
On Tue, Jun 26, 2018, 4:09 AM Emil Velikov wrote: > On 26 June 2018 at 01:48, Timothy Arceri wrote: > > On 26/06/18 09:51, Dieter Nützel wrote: > >> > >> Hello Marek, > >> > >> after this series landed I get this: > >> > >> Making all in targets/pipe-loader > >> make[4]: Verzeichnis „/opt/mesa/src/gallium/targets/pipe-loader“ wird > >> betreten > >>CXXLDpipe_r600.la > >> > >> > ../../../../src/gallium/winsys/radeon/drm/.libs/libradeonwinsys.a(radeon_drm_surface.o): > >> In function `radeon_winsys_surface_init': > >> /opt/mesa/src/gallium/winsys/radeon/drm/radeon_drm_surface.c:307: > >> undefined reference to `ac_compute_cmask' > >> collect2: error: ld returned 1 exit status > >> make[4]: *** [Makefile:970: pipe_r600.la] Fehler 1 > >> > >> Didn't have more time for digging, yet. > > > > > > r600 probably doesn't get linked to the amd common (ac) code that is > > normally just shared between radv and radeonsi. > > > Precisely. A quick and simple solution would be to duplicate the > function in winsys/radeon. > Since neither R300 nor R600 require AC [or the addrlib that gets > pulled], this might be the less invasive workaround. > > What do you guys think? > I agree with the duplication. Marek > > -Emil > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev