Series Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>
On Sat, Jun 7, 2014 at 11:47 AM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> Our existing functions, brw_math and brw_math2, had unclear roles:
>
> Gen4-5 used brw_math for both unary and binary math functions; it never
> used brw_math2. Since operands are already in message registers, this
> is reasonable.
>
> Gen6+ used brw_math for unary math functions, and brw_math2 for binary
> math functions, duplicating a lot of code. The only real difference was
> that brw_math used brw_null_reg() for src1.
>
> This patch improves brw_math2's assertions to allow both unary and
> binary operations, renames it to gen6_math(), and drops the Gen6+ code
> out of brw_math().
>
> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
> ---
> src/mesa/drivers/dri/i965/brw_eu.h | 2 +-
> src/mesa/drivers/dri/i965/brw_eu_emit.c | 78
> ++++++++----------------
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 24 ++------
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 24 +++-----
> 4 files changed, 39 insertions(+), 89 deletions(-)
>
> Deleting more pointless driver code, 124 lines at a time :)
>
> This series is available as the 'mathsplit' branch in my tree. I piglited
> it on Haswell and Sandybridge - no regressions.
>
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> b/src/mesa/drivers/dri/i965/brw_eu.h
> index c9e5a4b..0f8163d 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> @@ -285,7 +285,7 @@ void brw_math( struct brw_compile *p,
> unsigned data_type,
> unsigned precision );
>
> -void brw_math2(struct brw_compile *p,
> +void gen6_math(struct brw_compile *p,
> struct brw_reg dest,
> unsigned function,
> struct brw_reg src0,
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index b89070b..754d18d 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -1841,63 +1841,27 @@ void brw_math( struct brw_compile *p,
> unsigned precision )
> {
> struct brw_context *brw = p->brw;
> + struct brw_instruction *insn = next_insn(p, BRW_OPCODE_SEND);
>
> - if (brw->gen >= 6) {
> - struct brw_instruction *insn = next_insn(p, BRW_OPCODE_MATH);
> -
> - assert(dest.file == BRW_GENERAL_REGISTER_FILE ||
> - (brw->gen >= 7 && dest.file == BRW_MESSAGE_REGISTER_FILE));
> - assert(src.file == BRW_GENERAL_REGISTER_FILE);
> -
> - assert(dest.hstride == BRW_HORIZONTAL_STRIDE_1);
> - if (brw->gen == 6)
> - assert(src.hstride == BRW_HORIZONTAL_STRIDE_1);
> -
> - /* Source modifiers are ignored for extended math instructions on
> Gen6. */
> - if (brw->gen == 6) {
> - assert(!src.negate);
> - assert(!src.abs);
> - }
> -
> - if (function == BRW_MATH_FUNCTION_INT_DIV_QUOTIENT ||
> - function == BRW_MATH_FUNCTION_INT_DIV_REMAINDER ||
> - function == BRW_MATH_FUNCTION_INT_DIV_QUOTIENT_AND_REMAINDER) {
> - assert(src.type != BRW_REGISTER_TYPE_F);
> - } else {
> - assert(src.type == BRW_REGISTER_TYPE_F);
> - }
> -
> - /* Math is the same ISA format as other opcodes, except that
> CondModifier
> - * becomes FC[3:0] and ThreadCtrl becomes FC[5:4].
> - */
> - insn->header.destreg__conditionalmod = function;
> -
> - brw_set_dest(p, insn, dest);
> - brw_set_src0(p, insn, src);
> - brw_set_src1(p, insn, brw_null_reg());
> - } else {
> - struct brw_instruction *insn = next_insn(p, BRW_OPCODE_SEND);
> + assert(brw->gen < 6);
>
> - /* Example code doesn't set predicate_control for send
> - * instructions.
> - */
> - insn->header.predicate_control = 0;
> - insn->header.destreg__conditionalmod = msg_reg_nr;
> + /* Example code doesn't set predicate_control for send
> + * instructions.
> + */
> + insn->header.predicate_control = 0;
> + insn->header.destreg__conditionalmod = msg_reg_nr;
>
> - brw_set_dest(p, insn, dest);
> - brw_set_src0(p, insn, src);
> - brw_set_math_message(p,
> - insn,
> - function,
> - src.type == BRW_REGISTER_TYPE_D,
> - precision,
> - data_type);
> - }
> + brw_set_dest(p, insn, dest);
> + brw_set_src0(p, insn, src);
> + brw_set_math_message(p,
> + insn,
> + function,
> + src.type == BRW_REGISTER_TYPE_D,
> + precision,
> + data_type);
> }
>
> -/** Extended math function, float[8].
> - */
> -void brw_math2(struct brw_compile *p,
> +void gen6_math(struct brw_compile *p,
> struct brw_reg dest,
> unsigned function,
> struct brw_reg src0,
> @@ -1906,10 +1870,11 @@ void brw_math2(struct brw_compile *p,
> struct brw_context *brw = p->brw;
> struct brw_instruction *insn = next_insn(p, BRW_OPCODE_MATH);
>
> + assert(brw->gen >= 6);
> +
> assert(dest.file == BRW_GENERAL_REGISTER_FILE ||
> (brw->gen >= 7 && dest.file == BRW_MESSAGE_REGISTER_FILE));
> assert(src0.file == BRW_GENERAL_REGISTER_FILE);
> - assert(src1.file == BRW_GENERAL_REGISTER_FILE);
>
> assert(dest.hstride == BRW_HORIZONTAL_STRIDE_1);
> if (brw->gen == 6) {
> @@ -1922,9 +1887,16 @@ void brw_math2(struct brw_compile *p,
> function == BRW_MATH_FUNCTION_INT_DIV_QUOTIENT_AND_REMAINDER) {
> assert(src0.type != BRW_REGISTER_TYPE_F);
> assert(src1.type != BRW_REGISTER_TYPE_F);
> + assert(src1.file == BRW_GENERAL_REGISTER_FILE);
> } else {
> assert(src0.type == BRW_REGISTER_TYPE_F);
> assert(src1.type == BRW_REGISTER_TYPE_F);
> + if (function == BRW_MATH_FUNCTION_POW) {
> + assert(src1.file == BRW_GENERAL_REGISTER_FILE);
> + } else {
> + assert(src1.file == BRW_ARCHITECTURE_REGISTER_FILE &&
> + src1.nr == BRW_ARF_NULL);
> + }
> }
>
> /* Source modifiers are ignored for extended math instructions on Gen6. */
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 3ff7682..7097e67 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -260,11 +260,7 @@ fs_generator::generate_math1_gen7(fs_inst *inst,
> struct brw_reg src0)
> {
> assert(inst->mlen == 0);
> - brw_math(p, dst,
> - brw_math_function(inst->opcode),
> - 0, src0,
> - BRW_MATH_DATA_VECTOR,
> - BRW_MATH_PRECISION_FULL);
> + gen6_math(p, dst, brw_math_function(inst->opcode), src0, brw_null_reg());
> }
>
> void
> @@ -274,7 +270,7 @@ fs_generator::generate_math2_gen7(fs_inst *inst,
> struct brw_reg src1)
> {
> assert(inst->mlen == 0);
> - brw_math2(p, dst, brw_math_function(inst->opcode), src0, src1);
> + gen6_math(p, dst, brw_math_function(inst->opcode), src0, src1);
> }
>
> void
> @@ -287,19 +283,11 @@ fs_generator::generate_math1_gen6(fs_inst *inst,
> assert(inst->mlen == 0);
>
> brw_set_default_compression_control(p, BRW_COMPRESSION_NONE);
> - brw_math(p, dst,
> - op,
> - 0, src0,
> - BRW_MATH_DATA_VECTOR,
> - BRW_MATH_PRECISION_FULL);
> + gen6_math(p, dst, op, src0, brw_null_reg());
>
> if (dispatch_width == 16) {
> brw_set_default_compression_control(p, BRW_COMPRESSION_2NDHALF);
> - brw_math(p, sechalf(dst),
> - op,
> - 0, sechalf(src0),
> - BRW_MATH_DATA_VECTOR,
> - BRW_MATH_PRECISION_FULL);
> + gen6_math(p, sechalf(dst), op, sechalf(src0), brw_null_reg());
> brw_set_default_compression_control(p, BRW_COMPRESSION_COMPRESSED);
> }
> }
> @@ -315,11 +303,11 @@ fs_generator::generate_math2_gen6(fs_inst *inst,
> assert(inst->mlen == 0);
>
> brw_set_default_compression_control(p, BRW_COMPRESSION_NONE);
> - brw_math2(p, dst, op, src0, src1);
> + gen6_math(p, dst, op, src0, src1);
>
> if (dispatch_width == 16) {
> brw_set_default_compression_control(p, BRW_COMPRESSION_2NDHALF);
> - brw_math2(p, sechalf(dst), op, sechalf(src0), sechalf(src1));
> + gen6_math(p, sechalf(dst), op, sechalf(src0), sechalf(src1));
> brw_set_default_compression_control(p, BRW_COMPRESSION_COMPRESSED);
> }
> }
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index e95e6d9..931741f 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -186,13 +186,7 @@ vec4_generator::generate_math1_gen6(vec4_instruction
> *inst,
> check_gen6_math_src_arg(src);
>
> brw_set_default_access_mode(p, BRW_ALIGN_1);
> - brw_math(p,
> - dst,
> - brw_math_function(inst->opcode),
> - inst->base_mrf,
> - src,
> - BRW_MATH_DATA_SCALAR,
> - BRW_MATH_PRECISION_FULL);
> + gen6_math(p, dst, brw_math_function(inst->opcode), src, brw_null_reg());
> brw_set_default_access_mode(p, BRW_ALIGN_16);
> }
>
> @@ -202,10 +196,7 @@ vec4_generator::generate_math2_gen7(vec4_instruction
> *inst,
> struct brw_reg src0,
> struct brw_reg src1)
> {
> - brw_math2(p,
> - dst,
> - brw_math_function(inst->opcode),
> - src0, src1);
> + gen6_math(p, dst, brw_math_function(inst->opcode), src0, src1);
> }
>
> void
> @@ -221,10 +212,7 @@ vec4_generator::generate_math2_gen6(vec4_instruction
> *inst,
> check_gen6_math_src_arg(src1);
>
> brw_set_default_access_mode(p, BRW_ALIGN_1);
> - brw_math2(p,
> - dst,
> - brw_math_function(inst->opcode),
> - src0, src1);
> + gen6_math(p, dst, brw_math_function(inst->opcode), src0, src1);
> brw_set_default_access_mode(p, BRW_ALIGN_16);
> }
>
> @@ -1146,10 +1134,12 @@
> vec4_generator::generate_vec4_instruction(vec4_instruction *instruction,
> case SHADER_OPCODE_LOG2:
> case SHADER_OPCODE_SIN:
> case SHADER_OPCODE_COS:
> - if (brw->gen == 6) {
> + if (brw->gen >= 7) {
> + gen6_math(p, dst, brw_math_function(inst->opcode), src[0],
> + brw_null_reg());
> + } else if (brw->gen == 6) {
> generate_math1_gen6(inst, dst, src[0]);
> } else {
> - /* Also works for Gen7. */
> generate_math1_gen4(inst, dst, src[0]);
> }
> break;
> --
> 2.0.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
