On Tue, 2017-11-28 at 16:05 +0200, Andres Gomez wrote: > Nicolai, this looks like a good candidate to nominate for inclusion in ^^^^^^^ I meant Roland 😕
> 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
