Matt Turner <matts...@gmail.com> writes: > This reverts commit bbf8239f92ecd79431dfa41402e1c85318e7267f. > > I didn't like that commit to begin with -- computing things at compile > time is fine -- but for purposes of verifying that the resulting values > are correct, looking up 0x00 and 0x30 in a table is a lot better than > evaluating a recursive function. > FYI the function only ever recurses once in order to handle the sign bit orthogonally from the exponent and mantissa.
> Anyway, by making brw_imm_vf4() take the actual 8-bit restricted floats
> directly (instead of only integral values that would be converted to
> restricted float), we can use this function as a replacement for the
> vector float src_reg/fs_reg constructors.
Seems awful to me, it replaces a formula that can be verified correct by
reading the first paragraph of the description of the restricted float
format (Gen Graphics » BSpec » 3D-Media-GPGPU Engine » EU Overview » ISA
Introduction » EU Data Types » Numeric Data Types) with a table of magic
constants that don't give the slightest insight into the structure of
the restricted float format and therefore have to be verified
individually.
It also makes it impossible to use brw_imm_vf4() with
dynamically-specified integer constants which was the original
motivation of this change.
If the obfuscation is meant as an optimization, I don't think it helps.
GCC compiles the four int_to_float8() calls from brw_imm_vf4() into a
single instruction already when the arguments are constant expressions:
4a9dc9: 48 b8 00 00 00 00 30 movabs $0x3000000000,%rax
(assembly taken from brw_clip_interp_vertex())
> ---
> src/mesa/drivers/dri/i965/brw_clip_util.c | 2 +-
> src/mesa/drivers/dri/i965/brw_reg.h | 40
> ++++++++-----------------------
> 2 files changed, 11 insertions(+), 31 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_clip_util.c
> b/src/mesa/drivers/dri/i965/brw_clip_util.c
> index 40ad144..0253d52 100644
> --- a/src/mesa/drivers/dri/i965/brw_clip_util.c
> +++ b/src/mesa/drivers/dri/i965/brw_clip_util.c
> @@ -224,7 +224,7 @@ void brw_clip_interp_vertex( struct brw_clip_compile *c,
> vec1(t_nopersp),
> brw_imm_f(0));
> brw_IF(p, BRW_EXECUTE_1);
> - brw_MOV(p, t_nopersp, brw_imm_vf4(1, 0, 0, 0));
> + brw_MOV(p, t_nopersp, brw_imm_vf4(VF_ONE, VF_ZERO, VF_ZERO, VF_ZERO));
> brw_ENDIF(p);
>
> /* Now compute t_nopersp = t_nopersp.y/t_nopersp.x and broadcast it. */
> diff --git a/src/mesa/drivers/dri/i965/brw_reg.h
> b/src/mesa/drivers/dri/i965/brw_reg.h
> index b906892..a4c1901 100644
> --- a/src/mesa/drivers/dri/i965/brw_reg.h
> +++ b/src/mesa/drivers/dri/i965/brw_reg.h
> @@ -43,7 +43,6 @@
> #define BRW_REG_H
>
> #include <stdbool.h>
> -#include "main/imports.h"
> #include "main/compiler.h"
> #include "main/macros.h"
> #include "program/prog_instruction.h"
> @@ -638,38 +637,19 @@ brw_imm_vf(unsigned v)
> return imm;
> }
>
> -/**
> - * Convert an integer into a "restricted" 8-bit float, used in vector
> - * immediates. The 8-bit floating point format has a sign bit, an
> - * excess-3 3-bit exponent, and a 4-bit mantissa. All integer values
> - * from -31 to 31 can be represented exactly.
> - */
> -static inline uint8_t
> -int_to_float8(int x)
> -{
> - if (x == 0) {
> - return 0;
> - } else if (x < 0) {
> - return 1 << 7 | int_to_float8(-x);
> - } else {
> - const unsigned exponent = _mesa_logbase2(x);
> - const unsigned mantissa = (x - (1 << exponent)) << (4 - exponent);
> - assert(exponent <= 4);
> - return (exponent + 3) << 4 | mantissa;
> - }
> -}
> +#define VF_ZERO 0x0
> +#define VF_ONE 0x30
> +#define VF_NEG (1<<7)
>
> -/**
> - * Construct a floating-point packed vector immediate from its integer
> - * values. \sa int_to_float8()
> - */
> static inline struct brw_reg
> -brw_imm_vf4(int v0, int v1, int v2, int v3)
> +brw_imm_vf4(unsigned v0, unsigned v1, unsigned v2, unsigned v3)
> {
> - return brw_imm_vf((int_to_float8(v0) << 0) |
> - (int_to_float8(v1) << 8) |
> - (int_to_float8(v2) << 16) |
> - (int_to_float8(v3) << 24));
> + struct brw_reg imm = brw_imm_reg(BRW_REGISTER_TYPE_VF);
> + imm.vstride = BRW_VERTICAL_STRIDE_0;
> + imm.width = BRW_WIDTH_4;
> + imm.hstride = BRW_HORIZONTAL_STRIDE_1;
> + imm.ud = ((v0 << 0) | (v1 << 8) | (v2 << 16) | (v3 << 24));
> + return imm;
> }
>
>
> --
> 2.4.9
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
