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, srol...@vmware.com wrote: > From: Roland Scheidegger <srol...@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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
