[Mesa-dev] [PATCH 1/6] i965: Split Gen4-5 and Gen6+ MATH instruction emitters.

Jordan Justen jljusten at gmail.com
Sun Jun 8 15:49:08 PDT 2014


Series Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>

On Sat, Jun 7, 2014 at 11:47 AM, Kenneth Graunke <kenneth at 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 <kenneth at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list