[Mesa-dev] [PATCH 2/5] r600: use DX10_CLAMP bit in shader setup
Andres Gomez
agomez at igalia.com
Tue Nov 28 14:05:09 UTC 2017
Nicolai, this looks like a good candidate to nominate for inclusion in
all the stable queues.
What do you think?
On Thu, 2017-11-09 at 20:00 +0100, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
>
> The docs are not very concise in what this really does, however both
> Alex Deucher and Nicolai Hähnle suggested this only really affects instructions
> using the CLAMP output modifier, and I've confirmed that with the newly
> changed piglit isinf_and_isnan test.
> So, with this bit set, if an instruction has the CLAMP modifier bit (which
> clamps to [0,1]) set, then NaNs will be converted to zero, otherwise the result
> will be NaN.
> D3D10 would require this, glsl doesn't have modifiers (with mesa
> clamp(x,0,1) would get converted to such a modifier) coupled with a
> whatever-floats-your-boat specified NaN behavior, but the clamp behavior
> should probably always be used (this also matches what a decomposition into
> min(1.0, max(x, 0.0)) would do, if min/max also adhere to the ieee spec of
> picking the non-nan result).
> Some apps may in fact rely on this, as this prevents misrenderings in
> This War of Mine since using ieee muls
> (ce7a045feeef8cad155f1c9aa07f166e146e3d00), without having to use clamped
> rcp opcode, which would also fix this bug there.
> radeonsi also seems to set this bit nowadays if I see that righ (albeit the
> llvm amdgpu code comment now says "Make clamp modifier on NaN input returns 0"
> instead of "Do not clamp NAN to 0" since it was changed, which also looks
> a bit misleading).
>
> v2: set it in all shader stages.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103544
> ---
> src/gallium/drivers/r600/evergreen_state.c | 6 ++++++
> src/gallium/drivers/r600/r600_state.c | 9 +++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/src/gallium/drivers/r600/evergreen_state.c b/src/gallium/drivers/r600/evergreen_state.c
> index 96eb35a981..ef323bf4f6 100644
> --- a/src/gallium/drivers/r600/evergreen_state.c
> +++ b/src/gallium/drivers/r600/evergreen_state.c
> @@ -3235,6 +3235,7 @@ void evergreen_update_ps_state(struct pipe_context *ctx, struct r600_pipe_shader
> r600_store_value(cb, /* R_028844_SQ_PGM_RESOURCES_PS */
> S_028844_NUM_GPRS(rshader->bc.ngpr) |
> S_028844_PRIME_CACHE_ON_DRAW(1) |
> + S_028844_DX10_CLAMP(1) |
> S_028844_STACK_SIZE(rshader->bc.nstack));
> /* After that, the NOP relocation packet must be emitted (shader->bo, RADEON_USAGE_READ). */
>
> @@ -3255,6 +3256,7 @@ void evergreen_update_es_state(struct pipe_context *ctx, struct r600_pipe_shader
>
> r600_store_context_reg(cb, R_028890_SQ_PGM_RESOURCES_ES,
> S_028890_NUM_GPRS(rshader->bc.ngpr) |
> + S_028890_DX10_CLAMP(1) |
> S_028890_STACK_SIZE(rshader->bc.nstack));
> r600_store_context_reg(cb, R_02888C_SQ_PGM_START_ES,
> shader->bo->gpu_address >> 8);
> @@ -3317,6 +3319,7 @@ void evergreen_update_gs_state(struct pipe_context *ctx, struct r600_pipe_shader
>
> r600_store_context_reg(cb, R_028878_SQ_PGM_RESOURCES_GS,
> S_028878_NUM_GPRS(rshader->bc.ngpr) |
> + S_028878_DX10_CLAMP(1) |
> S_028878_STACK_SIZE(rshader->bc.nstack));
> r600_store_context_reg(cb, R_028874_SQ_PGM_START_GS,
> shader->bo->gpu_address >> 8);
> @@ -3357,6 +3360,7 @@ void evergreen_update_vs_state(struct pipe_context *ctx, struct r600_pipe_shader
> S_0286C4_VS_EXPORT_COUNT(nparams - 1));
> r600_store_context_reg(cb, R_028860_SQ_PGM_RESOURCES_VS,
> S_028860_NUM_GPRS(rshader->bc.ngpr) |
> + S_028860_DX10_CLAMP(1) |
> S_028860_STACK_SIZE(rshader->bc.nstack));
> if (rshader->vs_position_window_space) {
> r600_store_context_reg(cb, R_028818_PA_CL_VTE_CNTL,
> @@ -3391,6 +3395,7 @@ void evergreen_update_hs_state(struct pipe_context *ctx, struct r600_pipe_shader
> r600_init_command_buffer(cb, 32);
> r600_store_context_reg(cb, R_0288BC_SQ_PGM_RESOURCES_HS,
> S_0288BC_NUM_GPRS(rshader->bc.ngpr) |
> + S_0288BC_DX10_CLAMP(1) |
> S_0288BC_STACK_SIZE(rshader->bc.nstack));
> r600_store_context_reg(cb, R_0288B8_SQ_PGM_START_HS,
> shader->bo->gpu_address >> 8);
> @@ -3404,6 +3409,7 @@ void evergreen_update_ls_state(struct pipe_context *ctx, struct r600_pipe_shader
> r600_init_command_buffer(cb, 32);
> r600_store_context_reg(cb, R_0288D4_SQ_PGM_RESOURCES_LS,
> S_0288D4_NUM_GPRS(rshader->bc.ngpr) |
> + S_0288D4_DX10_CLAMP(1) |
> S_0288D4_STACK_SIZE(rshader->bc.nstack));
> r600_store_context_reg(cb, R_0288D0_SQ_PGM_START_LS,
> shader->bo->gpu_address >> 8);
> diff --git a/src/gallium/drivers/r600/r600_state.c b/src/gallium/drivers/r600/r600_state.c
> index c21e8dabb1..db3d6db70b 100644
> --- a/src/gallium/drivers/r600/r600_state.c
> +++ b/src/gallium/drivers/r600/r600_state.c
> @@ -2548,6 +2548,12 @@ void r600_update_ps_state(struct pipe_context *ctx, struct r600_pipe_shader *sha
> r600_store_context_reg_seq(cb, R_028850_SQ_PGM_RESOURCES_PS, 2);
> r600_store_value(cb, /* R_028850_SQ_PGM_RESOURCES_PS*/
> S_028850_NUM_GPRS(rshader->bc.ngpr) |
> + /*
> + * docs are misleading about the dx10_clamp bit. This only affects
> + * instructions using CLAMP dst modifier, in which case they will
> + * return 0 with this set for a NaN (otherwise NaN).
> + */
> + S_028850_DX10_CLAMP(1) |
> S_028850_STACK_SIZE(rshader->bc.nstack) |
> S_028850_UNCACHED_FIRST_INST(ufi));
> r600_store_value(cb, exports_ps); /* R_028854_SQ_PGM_EXPORTS_PS */
> @@ -2597,6 +2603,7 @@ void r600_update_vs_state(struct pipe_context *ctx, struct r600_pipe_shader *sha
> S_0286C4_VS_EXPORT_COUNT(nparams - 1));
> r600_store_context_reg(cb, R_028868_SQ_PGM_RESOURCES_VS,
> S_028868_NUM_GPRS(rshader->bc.ngpr) |
> + S_028868_DX10_CLAMP(1) |
> S_028868_STACK_SIZE(rshader->bc.nstack));
> if (rshader->vs_position_window_space) {
> r600_store_context_reg(cb, R_028818_PA_CL_VTE_CNTL,
> @@ -2681,6 +2688,7 @@ void r600_update_gs_state(struct pipe_context *ctx, struct r600_pipe_shader *sha
>
> r600_store_context_reg(cb, R_02887C_SQ_PGM_RESOURCES_GS,
> S_02887C_NUM_GPRS(rshader->bc.ngpr) |
> + S_02887C_DX10_CLAMP(1) |
> S_02887C_STACK_SIZE(rshader->bc.nstack));
> r600_store_context_reg(cb, R_02886C_SQ_PGM_START_GS, 0);
> /* After that, the NOP relocation packet must be emitted (shader->bo, RADEON_USAGE_READ). */
> @@ -2695,6 +2703,7 @@ void r600_update_es_state(struct pipe_context *ctx, struct r600_pipe_shader *sha
>
> r600_store_context_reg(cb, R_028890_SQ_PGM_RESOURCES_ES,
> S_028890_NUM_GPRS(rshader->bc.ngpr) |
> + S_028890_DX10_CLAMP(1) |
> S_028890_STACK_SIZE(rshader->bc.nstack));
> r600_store_context_reg(cb, R_028880_SQ_PGM_START_ES, 0);
> /* After that, the NOP relocation packet must be emitted (shader->bo, RADEON_USAGE_READ). */
--
Br,
Andres
More information about the mesa-dev
mailing list