On 12/16/2015 01:40 AM, Ilia Mirkin wrote:
Hardly a complete review, but a handful of comments:
Thank you for your comments. See my replies inline.
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
Fixed.
+ }
+ 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
I'd love to, but there is no such TGSI opcode.
+ }
+ 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
Yes, thanks. It was too far down, I didn't find it.
The other one should be expressible as CMP as well I think.
Ok, I changed it to tmp=SUB(0.5, arg2); dst=CMP(tmp, arg0, arg1)
+ } 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
Fixed. Initially I used TGSI_OPCODE_CLAMP below this block, but then I
discovered the saturate modifier, and the conditional somehow escaped my
attention.
+ 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?
(reply to the whole thread on this)
I took a brief look on NIR and it seems much more complicated than TGSI.
I'd rather wait for the end of the IR-wars before taking action.
Meanwhile, the Intel GPUs can support this extension with ilo, or the
TGSI->NIR translator.
MM
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
