On Mon, Feb 8, 2016 at 5:08 PM, Tom Stellard <t...@stellard.net> wrote: > On Sat, Feb 06, 2016 at 01:15:42PM +0100, Marek Olšák wrote: >> From: Marek Olšák <marek.ol...@amd.com> >> >> This fixes FP16 conversion instructions for VI, which has 16-bit floats, >> but not SI & CI, which can't disable denorms for those instructions. > > Do you know why this fixes FP16 conversions? What does the OpenGL > spec say about denormal handing?
Yes, I know why. The patch explain everything as far as I can see though. What isn't clear? SI & CI: Don't support FP16. FP16 conversions are hardcoded to emit and accept FP16 denormals. VI: Supports FP16. FP16 denormal support is now configurable and affects FP16 conversions as well.(shared setting with FP64). OpenGL doesn't require denormals. Piglit does. I think this is incorrect piglit behavior. > >> --- >> src/gallium/drivers/radeonsi/si_shader.c | 14 ++++++++++++++ >> src/gallium/drivers/radeonsi/si_state_shaders.c | 18 ++++++++++++------ >> src/gallium/drivers/radeonsi/sid.h | 3 +++ >> 3 files changed, 29 insertions(+), 6 deletions(-) >> >> diff --git a/src/gallium/drivers/radeonsi/si_shader.c >> b/src/gallium/drivers/radeonsi/si_shader.c >> index a4680ce..3f1db70 100644 >> --- a/src/gallium/drivers/radeonsi/si_shader.c >> +++ b/src/gallium/drivers/radeonsi/si_shader.c >> @@ -4155,6 +4155,20 @@ int si_compile_llvm(struct si_screen *sscreen, >> >> si_shader_binary_read_config(binary, conf, 0); >> >> + /* Enable 64-bit and 16-bit denormals, because there is no performance >> + * cost. >> + * >> + * If denormals are enabled, all floating-point output modifiers are >> + * ignored. >> + * >> + * Don't enable denormals for 32-bit floats, because: >> + * - Floating-point output modifiers would be ignored by the hw. >> + * - Some opcodes don't support denormals, such as v_mad_f32. We would >> + * have to stop using those. >> + * - SI & CI would be very slow. >> + */ >> + conf->float_mode |= V_00B028_FP_64_DENORMS; >> + > > Do SI/CI support fp64 denorms? If so, won't this hurt performance? Yes, they do. Fp64 denorms don't hurt performance. Only fp32 denorms do on SI & CI. > > We should tell the compiler we are enabling fp-64 denorms by adding > +fp64-denormals to the feature string. It would also be better to > read the float_mode value from the config registers emitted by the > compiler. Yes, I agree, but LLVM only sets these parameters for compute or even HSA-only kernels, not for graphics shaders. We need to set the mode for all users _now_, not in 6 months. Last time I looked, +fp64-denormals had no effect on graphics shaders. Marek _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
