On Tue, Dec 15, 2015 at 7:59 PM, Ian Romanick <i...@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é <mtm...@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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev