[Mesa-dev] [PATCH 03/11] st/mesa: implement GL_ATI_fragment_shader

Ilia Mirkin imirkin at alum.mit.edu
Tue Dec 15 17:08:48 PST 2015


On Tue, Dec 15, 2015 at 7:59 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 12/15/2015 04:40 PM, Ilia Mirkin wrote:
>> Hardly a complete review, but a handful of comments:
>>
>> On Tue, Dec 15, 2015 at 6:05 PM, Miklós Máté <mtmkls at gmail.com> wrote:
>>> ---
>>>  src/mesa/Makefile.sources                 |   1 +
>>>  src/mesa/state_tracker/st_atifs_to_tgsi.c | 798 ++++++++++++++++++++++++++++++
>>>  src/mesa/state_tracker/st_atifs_to_tgsi.h |  49 ++
>>>  src/mesa/state_tracker/st_atom_constbuf.c |  14 +
>>>  src/mesa/state_tracker/st_cb_drawpixels.c |   1 +
>>>  src/mesa/state_tracker/st_cb_program.c    |  35 +-
>>>  src/mesa/state_tracker/st_program.c       |  22 +
>>>  src/mesa/state_tracker/st_program.h       |   1 +
>>>  8 files changed, 920 insertions(+), 1 deletion(-)
>>>  create mode 100644 src/mesa/state_tracker/st_atifs_to_tgsi.c
>>>  create mode 100644 src/mesa/state_tracker/st_atifs_to_tgsi.h
>>>
>>> +static struct ureg_src prepare_argument(struct st_translate *t, const unsigned argId,
>>> +      const struct atifragshader_src_register *srcReg)
>>> +{
>>> +   struct ureg_src src = get_source(t, srcReg->Index);
>>> +   struct ureg_dst arg = get_temp(t, MAX_NUM_FRAGMENT_REGISTERS_ATI+argId);
>>> +
>>> +   switch (srcReg->argRep) {
>>> +      case GL_NONE:
>>> +         break;
>>> +      case GL_RED:
>>> +         src = ureg_swizzle(src,
>>> +               TGSI_SWIZZLE_X, TGSI_SWIZZLE_X, TGSI_SWIZZLE_X, TGSI_SWIZZLE_X);
>>> +         break;
>>> +      case GL_GREEN:
>>> +         src = ureg_swizzle(src,
>>> +               TGSI_SWIZZLE_Y, TGSI_SWIZZLE_Y, TGSI_SWIZZLE_Y, TGSI_SWIZZLE_Y);
>>> +         break;
>>> +      case GL_BLUE:
>>> +         src = ureg_swizzle(src,
>>> +               TGSI_SWIZZLE_Z, TGSI_SWIZZLE_Z, TGSI_SWIZZLE_Z, TGSI_SWIZZLE_Z);
>>> +         break;
>>> +      case GL_ALPHA:
>>> +         src = ureg_swizzle(src,
>>> +               TGSI_SWIZZLE_W, TGSI_SWIZZLE_W, TGSI_SWIZZLE_W, TGSI_SWIZZLE_W);
>>> +         break;
>>> +   }
>>> +   emit_insn(t, TGSI_OPCODE_MOV, &arg, 1, &src, 1);
>>> +
>>> +   if (srcReg->argMod & GL_COMP_BIT_ATI) {
>>> +      struct ureg_src modsrc[2];
>>> +      modsrc[0] = ureg_imm1f(t->ureg, 1.0);
>>> +      modsrc[1] = ureg_src(arg);
>>> +
>>> +      emit_insn(t, TGSI_OPCODE_SUB, &arg, 1, modsrc, 2);
>>> +   }
>>> +   if (srcReg->argMod & GL_BIAS_BIT_ATI) {
>>> +      struct ureg_src modsrc[2];
>>> +      modsrc[0] = ureg_src(arg);
>>> +      modsrc[1] = ureg_imm1f(t->ureg, 0.5);
>>> +
>>> +      emit_insn(t, TGSI_OPCODE_SUB, &arg, 1, modsrc, 2);
>>> +   }
>>> +   if (srcReg->argMod & GL_2X_BIT_ATI) {
>>> +      struct ureg_src modsrc[2];
>>> +      modsrc[0] = ureg_src(arg);
>>> +      modsrc[1] = ureg_imm1f(t->ureg, 2.0);
>>> +
>>> +      emit_insn(t, TGSI_OPCODE_MUL, &arg, 1, modsrc, 2);
>>
>> aka ADD arg, arg, arg
>>
>>> +   }
>>> +   if (srcReg->argMod & GL_NEGATE_BIT_ATI) {
>>> +      struct ureg_src modsrc[2];
>>> +      modsrc[0] = ureg_src(arg);
>>> +      modsrc[1] = ureg_imm1f(t->ureg, -1.0);
>>> +
>>> +      emit_insn(t, TGSI_OPCODE_MUL, &arg, 1, modsrc, 2);
>>
>> aka NEG arg, arg
>>
>>> +   }
>>> +   return  ureg_src(arg);
>>> +}
>>> +
>>> +/* These instructions have no direct equivalent in TGSI */
>>> +static void emit_special_inst(struct st_translate *t, struct instruction_desc *desc,
>>> +      struct ureg_dst *dst, struct ureg_src *args, unsigned argcount)
>>> +{
>>> +   struct ureg_dst tmp[1];
>>> +   struct ureg_src src[3];
>>> +
>>> +   if        (desc->special == 1) {
>>> +      tmp[0] = get_temp(t, MAX_NUM_FRAGMENT_REGISTERS_ATI+2); // re-purpose a3
>>> +      src[0] = ureg_imm1f(t->ureg, 0.5f);
>>> +      src[1] = args[2];
>>> +      emit_insn(t, TGSI_OPCODE_SLT, tmp, 1, src, 2);
>>> +      src[0] = ureg_src(tmp[0]);
>>> +      src[1] = args[0];
>>> +      src[2] = args[1];
>>> +      emit_insn(t, TGSI_OPCODE_LRP, dst, 1, src, 3);
>>> +   } else if (desc->special == 2) {
>>> +      tmp[0] = get_temp(t, MAX_NUM_FRAGMENT_REGISTERS_ATI+2); // re-purpose a3
>>> +      src[0] = args[2];
>>> +      src[1] = ureg_imm1f(t->ureg, 0.0f);
>>> +      emit_insn(t, TGSI_OPCODE_SGE, tmp, 1, src, 2);
>>> +      src[0] = ureg_src(tmp[0]);
>>> +      src[1] = args[0];
>>> +      src[2] = args[1];
>>> +      emit_insn(t, TGSI_OPCODE_LRP, dst, 1, src, 3);
>>
>> Isn't this the CMP instruction? Just flip the args.
>>
>> http://gallium.readthedocs.org/en/latest/tgsi.html#opcode-CMP
>>
>> The other one should be expressible as CMP as well I think.
>>
>>> +   } else if (desc->special == 3) {
>>> +      src[0] = args[0];
>>> +      src[1] = args[1];
>>> +      src[2] = ureg_swizzle(args[2],
>>> +            TGSI_SWIZZLE_Z, TGSI_SWIZZLE_Z, TGSI_SWIZZLE_Z, TGSI_SWIZZLE_Z);
>>> +      emit_insn(t, TGSI_OPCODE_DP2A, dst, 1, src, 3);
>>> +   }
>>> +}
>>> +
>>> +static void emit_arith_inst(struct st_translate *t,
>>> +      struct instruction_desc *desc,
>>> +      struct ureg_dst *dst, struct ureg_src *args, unsigned argcount)
>>> +{
>>> +   if (desc->special) {
>>> +      return emit_special_inst(t, desc, dst, args, argcount);
>>> +   }
>>> +
>>> +   emit_insn(t, desc->TGSI_opcode, dst, 1, args, argcount);
>>> +}
>>> +
>>> +static void emit_dstmod(struct st_translate *t,
>>> +      struct ureg_dst dst, GLuint dstMod)
>>> +{
>>> +   float imm = 0.0;
>>
>> 1.0 right? (if you just have the saturate bit)
>>
>>> +   struct ureg_src src[3];
>>> +
>>> +   if (dstMod == GL_NONE) {
>>> +      return;
>>> +   }
>>> +
>>> +   if        (dstMod & GL_2X_BIT_ATI) {
>>> +      imm = 2.0f;
>>> +   } else if (dstMod & GL_4X_BIT_ATI) {
>>> +      imm = 4.0f;
>>> +   } else if (dstMod & GL_8X_BIT_ATI) {
>>> +      imm = 8.0f;
>>> +   } else if (dstMod & GL_HALF_BIT_ATI) {
>>> +      imm = 0.5f;
>>> +   } else if (dstMod & GL_QUARTER_BIT_ATI) {
>>> +      imm = 0.25f;
>>> +   } else if (dstMod & GL_EIGHTH_BIT_ATI) {
>>> +      imm = 0.125f;
>>> +   }
>>> +   if (imm) {
>>
>> and this should be unconditional
>>
>>> +      src[0] = ureg_src(dst);
>>> +      src[1] = ureg_imm1f(t->ureg, imm);
>>> +      if (dstMod & GL_SATURATE_BIT_ATI) {
>>> +         dst = ureg_saturate(dst);
>>> +      }
>>> +      emit_insn(t, TGSI_OPCODE_MUL, &dst, 1, src, 2);
>>> +   }
>>> +}
>>
>> More generally, all this seems like pretty simple stuff (in terms of
>> instruction fanciness) -- would it have been better to try to convert
>> this to mesa ir instead, to enable this to be used on i965 as well,
>> which can consume mesa ir?
>
> I was going to ask about going through NIR.  We have some intention to
> get rid of Mesa eventually, and we already translate Mesa IR to NIR for
> i965.

Then there would be 2 separate converters, which seems undesirable. I
was suggesting using mesa IR to reduce that burden.

  -ilia


More information about the mesa-dev mailing list