[Mesa-dev] [PATCH 03/11] st/mesa: implement GL_ATI_fragment_shader
Ilia Mirkin
imirkin at alum.mit.edu
Tue Dec 15 18:19:00 PST 2015
On Dec 15, 2015 8:59 PM, "Ian Romanick" <idr at freedesktop.org> wrote:
>
> On 12/15/2015 05:08 PM, Ilia Mirkin wrote:
> > 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.
>
> But we can already do NIR -> TGSI, right? So isn't it still just one
> converter? It also seems undesirable to add a bunch of code that will
> just need to get rewritten in the near future. Since I've already
> signed up to do a bunch of the Mesa IR removal, I'd rather wait to get
> this extension in i965 than add more work to that particular task. The
> first thing I'd have to do is... convert the ATIfs->Mesa IR converter to
> ATIfs->NIR. :)
Similarly we can do tgsi -> nir. I wouldn't want to sign a new contributor
up for working all that out, so if you feel like Mesa ir is on its way out,
ignore my suggestion entirely and lets keep this as tgsi only.
>
> > -ilia
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151215/a11d1c50/attachment.html>
More information about the mesa-dev
mailing list