From 1f345bc7d62fac1773afef7310174ad209ae986b Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 24 Apr 2019 01:15:15 +0000 Subject: [PATCH] panfrost/midgard: Refactor opcode tables We create an all-encompassing opcode table for handling name and properties, removing a number of ad hoc opcode tables which became brittle and quickly out of date. While we're at it, we fix some incorrect opcodes relating to ball/bany, and move a small function out to midgard_compile.c. Together these changes should allow compilation without warnings, along with helping the codebase health considerably. Signed-off-by: Alyssa Rosenzweig --- .../drivers/panfrost/midgard/disassemble.c | 6 +- .../drivers/panfrost/midgard/helpers.h | 239 ++++++++---------- .../drivers/panfrost/midgard/midgard.h | 90 +------ .../panfrost/midgard/midgard_compile.c | 16 +- 4 files changed, 124 insertions(+), 227 deletions(-) diff --git a/src/gallium/drivers/panfrost/midgard/disassemble.c b/src/gallium/drivers/panfrost/midgard/disassemble.c index be0d250f7e5..719f5780863 100644 --- a/src/gallium/drivers/panfrost/midgard/disassemble.c +++ b/src/gallium/drivers/panfrost/midgard/disassemble.c @@ -44,10 +44,10 @@ print_alu_opcode(midgard_alu_op op) { bool int_op = false; - if (alu_opcode_names[op]) { - printf("%s", alu_opcode_names[op]); + if (alu_opcode_props[op].name) { + printf("%s", alu_opcode_props[op].name); - int_op = alu_opcode_names[op][0] == 'i'; + int_op = midgard_is_integer_op(op); } else printf("alu_op_%02X", op); diff --git a/src/gallium/drivers/panfrost/midgard/helpers.h b/src/gallium/drivers/panfrost/midgard/helpers.h index 24f58b5ec63..40923f287d0 100644 --- a/src/gallium/drivers/panfrost/midgard/helpers.h +++ b/src/gallium/drivers/panfrost/midgard/helpers.h @@ -122,67 +122,6 @@ #define LDST_NOP (3) -/* Is this opcode that of an integer (regardless of signedness)? */ - -static bool -midgard_is_integer_op(int op) -{ - switch (op) { - case midgard_alu_op_iadd: - case midgard_alu_op_ishladd: - case midgard_alu_op_isub: - case midgard_alu_op_imul: - case midgard_alu_op_imin: - case midgard_alu_op_umin: - case midgard_alu_op_imax: - case midgard_alu_op_umax: - case midgard_alu_op_iasr: - case midgard_alu_op_ilsr: - case midgard_alu_op_ishl: - case midgard_alu_op_iand: - case midgard_alu_op_ior: - case midgard_alu_op_inot: - case midgard_alu_op_iandnot: - case midgard_alu_op_ixor: - case midgard_alu_op_ilzcnt: - case midgard_alu_op_ibitcount8: - case midgard_alu_op_imov: - case midgard_alu_op_iabs: - case midgard_alu_op_ieq: - case midgard_alu_op_ine: - case midgard_alu_op_ult: - case midgard_alu_op_ule: - case midgard_alu_op_ilt: - case midgard_alu_op_ile: - case midgard_alu_op_iball_eq: - case midgard_alu_op_ball: - case midgard_alu_op_uball_lt: - case midgard_alu_op_uball_lte: - case midgard_alu_op_iball_lt: - case midgard_alu_op_iball_lte: - case midgard_alu_op_ibany_eq: - case midgard_alu_op_ibany_neq: - case midgard_alu_op_ubany_lt: - case midgard_alu_op_ubany_lte: - case midgard_alu_op_ibany_lt: - case midgard_alu_op_ibany_lte: - case midgard_alu_op_i2f: - case midgard_alu_op_u2f: - case midgard_alu_op_icsel: - return true; - - default: - return false; - } -} - -/* Is this unit a branch? */ -static bool -midgard_is_branch_unit(unsigned unit) -{ - return (unit == ALU_ENAB_BRANCH) || (unit == ALU_ENAB_BR_COMPACT); -} - /* There are five ALU units: VMUL, VADD, SMUL, SADD, LUT. A given opcode is * implemented on some subset of these units (or occassionally all of them). * This table encodes a bit mask of valid units for each opcode, so the @@ -205,80 +144,120 @@ midgard_is_branch_unit(unsigned unit) #define UNITS_VECTOR (UNIT_VMUL | UNIT_VADD) #define UNITS_ANY_VECTOR (UNITS_VECTOR | UNIT_VLUT) -static unsigned alu_opcode_props[256] = { - [midgard_alu_op_fadd] = UNITS_ADD, - [midgard_alu_op_fmul] = UNITS_MUL | UNIT_VLUT, - [midgard_alu_op_fmin] = UNITS_MUL | UNITS_ADD, - [midgard_alu_op_fmax] = UNITS_MUL | UNITS_ADD, - [midgard_alu_op_imin] = UNITS_MOST, - [midgard_alu_op_imax] = UNITS_MOST, - [midgard_alu_op_umin] = UNITS_MOST, - [midgard_alu_op_umax] = UNITS_MOST, - [midgard_alu_op_fmov] = UNITS_ALL | QUIRK_FLIPPED_R24, - [midgard_alu_op_fround] = UNITS_ADD, - [midgard_alu_op_froundeven] = UNITS_ADD, - [midgard_alu_op_ftrunc] = UNITS_ADD, - [midgard_alu_op_ffloor] = UNITS_ADD, - [midgard_alu_op_fceil] = UNITS_ADD, - [midgard_alu_op_ffma] = UNIT_VLUT, +/* Table of mapping opcodes to accompanying properties relevant to + * scheduling/emission/etc */ + +static struct { + const char *name; + unsigned props; +} alu_opcode_props[256] = { + [midgard_alu_op_fadd] = {"fadd", UNITS_ADD}, + [midgard_alu_op_fmul] = {"fmul", UNITS_MUL | UNIT_VLUT}, + [midgard_alu_op_fmin] = {"fmin", UNITS_MUL | UNITS_ADD}, + [midgard_alu_op_fmax] = {"fmax", UNITS_MUL | UNITS_ADD}, + [midgard_alu_op_imin] = {"imin", UNITS_MOST}, + [midgard_alu_op_imax] = {"imax", UNITS_MOST}, + [midgard_alu_op_umin] = {"umin", UNITS_MOST}, + [midgard_alu_op_umax] = {"umax", UNITS_MOST}, + [midgard_alu_op_fmov] = {"fmov", UNITS_ALL | QUIRK_FLIPPED_R24}, + [midgard_alu_op_fround] = {"fround", UNITS_ADD}, + [midgard_alu_op_froundeven] = {"froundeven", UNITS_ADD}, + [midgard_alu_op_ftrunc] = {"ftrunc", UNITS_ADD}, + [midgard_alu_op_ffloor] = {"ffloor", UNITS_ADD}, + [midgard_alu_op_fceil] = {"fceil", UNITS_ADD}, + [midgard_alu_op_ffma] = {"ffma", UNIT_VLUT}, /* Though they output a scalar, they need to run on a vector unit * since they process vectors */ - [midgard_alu_op_fdot3] = UNIT_VMUL | OP_CHANNEL_COUNT(3), - [midgard_alu_op_fdot4] = UNIT_VMUL | OP_CHANNEL_COUNT(4), + [midgard_alu_op_fdot3] = {"fdot3", UNIT_VMUL | OP_CHANNEL_COUNT(3)}, + [midgard_alu_op_fdot3r] = {"fdot3r", UNIT_VMUL | OP_CHANNEL_COUNT(3)}, + [midgard_alu_op_fdot4] = {"fdot4", UNIT_VMUL | OP_CHANNEL_COUNT(4)}, /* Incredibly, iadd can run on vmul, etc */ - [midgard_alu_op_iadd] = UNITS_MOST, - [midgard_alu_op_iabs] = UNITS_MOST, - [midgard_alu_op_isub] = UNITS_MOST, - [midgard_alu_op_imul] = UNITS_MUL, - [midgard_alu_op_imov] = UNITS_MOST | QUIRK_FLIPPED_R24, + [midgard_alu_op_iadd] = {"iadd", UNITS_MOST}, + [midgard_alu_op_iabs] = {"iabs", UNITS_MOST}, + [midgard_alu_op_isub] = {"isub", UNITS_MOST}, + [midgard_alu_op_imul] = {"imul", UNITS_MUL}, + [midgard_alu_op_imov] = {"imov", UNITS_MOST | QUIRK_FLIPPED_R24}, /* For vector comparisons, use ball etc */ - [midgard_alu_op_feq] = UNITS_MOST, - [midgard_alu_op_fne] = UNITS_MOST, - [midgard_alu_op_fle] = UNITS_MOST, - [midgard_alu_op_flt] = UNITS_MOST, - [midgard_alu_op_ieq] = UNITS_MOST, - [midgard_alu_op_ine] = UNITS_MOST, - [midgard_alu_op_ilt] = UNITS_MOST, - [midgard_alu_op_ile] = UNITS_MOST, - [midgard_alu_op_ule] = UNITS_MOST, - [midgard_alu_op_ult] = UNITS_MOST, - - [midgard_alu_op_icsel] = UNITS_ADD, - [midgard_alu_op_fcsel_i] = UNITS_ADD, - [midgard_alu_op_fcsel] = UNITS_ADD | UNIT_SMUL, - - [midgard_alu_op_frcp] = UNIT_VLUT, - [midgard_alu_op_frsqrt] = UNIT_VLUT, - [midgard_alu_op_fsqrt] = UNIT_VLUT, - [midgard_alu_op_fpow_pt1] = UNIT_VLUT, - [midgard_alu_op_fexp2] = UNIT_VLUT, - [midgard_alu_op_flog2] = UNIT_VLUT, - - [midgard_alu_op_f2i] = UNITS_ADD, - [midgard_alu_op_f2u] = UNITS_ADD, - [midgard_alu_op_f2u8] = UNITS_ADD, - [midgard_alu_op_i2f] = UNITS_ADD, - [midgard_alu_op_u2f] = UNITS_ADD, - - [midgard_alu_op_fsin] = UNIT_VLUT, - [midgard_alu_op_fcos] = UNIT_VLUT, - - [midgard_alu_op_iand] = UNITS_ADD, /* XXX: Test case where it's right on smul but not sadd */ - [midgard_alu_op_ior] = UNITS_ADD, - [midgard_alu_op_ixor] = UNITS_ADD, - [midgard_alu_op_ilzcnt] = UNITS_ADD, - [midgard_alu_op_ibitcount8] = UNITS_ADD, - [midgard_alu_op_inot] = UNITS_MOST, - [midgard_alu_op_ishl] = UNITS_ADD, - [midgard_alu_op_iasr] = UNITS_ADD, - [midgard_alu_op_ilsr] = UNITS_ADD, - [midgard_alu_op_ilsr] = UNITS_ADD, - - [midgard_alu_op_fball_eq] = UNITS_VECTOR, - [midgard_alu_op_fbany_neq] = UNITS_VECTOR, - [midgard_alu_op_iball_eq] = UNITS_VECTOR, - [midgard_alu_op_ibany_neq] = UNITS_VECTOR + [midgard_alu_op_feq] = {"feq", UNITS_MOST}, + [midgard_alu_op_fne] = {"fne", UNITS_MOST}, + [midgard_alu_op_fle] = {"fle", UNITS_MOST}, + [midgard_alu_op_flt] = {"flt", UNITS_MOST}, + [midgard_alu_op_ieq] = {"ieq", UNITS_MOST}, + [midgard_alu_op_ine] = {"ine", UNITS_MOST}, + [midgard_alu_op_ilt] = {"ilt", UNITS_MOST}, + [midgard_alu_op_ile] = {"ile", UNITS_MOST}, + [midgard_alu_op_ult] = {"ult", UNITS_MOST}, + [midgard_alu_op_ule] = {"ule", UNITS_MOST}, + + [midgard_alu_op_icsel] = {"icsel", UNITS_ADD}, + [midgard_alu_op_fcsel_i] = {"fcsel_i", UNITS_ADD}, + [midgard_alu_op_fcsel] = {"fcsel", UNITS_ADD | UNIT_SMUL}, + + [midgard_alu_op_frcp] = {"frcp", UNIT_VLUT}, + [midgard_alu_op_frsqrt] = {"frsqrt", UNIT_VLUT}, + [midgard_alu_op_fsqrt] = {"fsqrt", UNIT_VLUT}, + [midgard_alu_op_fpow_pt1] = {"fpow_pt1", UNIT_VLUT}, + [midgard_alu_op_fexp2] = {"fexp2", UNIT_VLUT}, + [midgard_alu_op_flog2] = {"flog2", UNIT_VLUT}, + + [midgard_alu_op_f2i] = {"f2i", UNITS_ADD}, + [midgard_alu_op_f2u] = {"f2u", UNITS_ADD}, + [midgard_alu_op_f2u8] = {"f2u8", UNITS_ADD}, + [midgard_alu_op_i2f] = {"i2f", UNITS_ADD}, + [midgard_alu_op_u2f] = {"u2f", UNITS_ADD}, + + [midgard_alu_op_fsin] = {"fsin", UNIT_VLUT}, + [midgard_alu_op_fcos] = {"fcos", UNIT_VLUT}, + + /* XXX: Test case where it's right on smul but not sadd */ + [midgard_alu_op_iand] = {"iand", UNITS_ADD}, + + [midgard_alu_op_ior] = {"ior", UNITS_ADD}, + [midgard_alu_op_ixor] = {"ixor", UNITS_ADD}, + [midgard_alu_op_ilzcnt] = {"ilzcnt", UNITS_ADD}, + [midgard_alu_op_ibitcount8] = {"ibitcount8", UNITS_ADD}, + [midgard_alu_op_inot] = {"inot", UNITS_MOST}, + [midgard_alu_op_ishl] = {"ishl", UNITS_ADD}, + [midgard_alu_op_iasr] = {"iasr", UNITS_ADD}, + [midgard_alu_op_ilsr] = {"ilsr", UNITS_ADD}, + + [midgard_alu_op_fball_eq] = {"fball_eq", UNITS_VECTOR}, + [midgard_alu_op_fbany_neq] = {"fbany_neq", UNITS_VECTOR}, + [midgard_alu_op_iball_eq] = {"iball_eq", UNITS_VECTOR}, + [midgard_alu_op_iball_neq] = {"iball_neq", UNITS_VECTOR}, + [midgard_alu_op_ibany_eq] = {"ibany_eq", UNITS_VECTOR}, + [midgard_alu_op_ibany_neq] = {"ibany_neq", UNITS_VECTOR}, + + /* These instructions are not yet emitted by the compiler, so + * don't speculate about units yet */ + [midgard_alu_op_ishladd] = {"ishladd", 0}, + [midgard_alu_op_iandnot] = {"iandnot", 0}, + + [midgard_alu_op_uball_lt] = {"uball_lt", 0}, + [midgard_alu_op_uball_lte] = {"uball_lte", 0}, + [midgard_alu_op_iball_lt] = {"iball_lt", 0}, + [midgard_alu_op_iball_lte] = {"iball_lte", 0}, + [midgard_alu_op_ubany_lt] = {"ubany_lt", 0}, + [midgard_alu_op_ubany_lte] = {"ubany_lte", 0}, + [midgard_alu_op_ibany_lt] = {"ibany_lt", 0}, + [midgard_alu_op_ibany_lte] = {"ibany_lte", 0}, + + [midgard_alu_op_freduce] = {"freduce", 0}, + [midgard_alu_op_bball_eq] = {"bball_eq", 0}, + [midgard_alu_op_bbany_neq] = {"bball_eq", 0}, + [midgard_alu_op_fatan2_pt1] = {"fatan2_pt1", 0}, + [midgard_alu_op_fatan_pt2] = {"fatan_pt2", 0}, }; + +/* Is this opcode that of an integer (regardless of signedness)? Instruction + * names authoritatively determine types */ + +static bool +midgard_is_integer_op(int op) +{ + char prefix = alu_opcode_props[op].name[0]; + return (prefix == 'i') || (prefix == 'u'); +} diff --git a/src/gallium/drivers/panfrost/midgard/midgard.h b/src/gallium/drivers/panfrost/midgard/midgard.h index f426b9712b1..c7cc5d44d1c 100644 --- a/src/gallium/drivers/panfrost/midgard/midgard.h +++ b/src/gallium/drivers/panfrost/midgard/midgard.h @@ -112,7 +112,7 @@ typedef enum { midgard_alu_op_ilt = 0xA4, midgard_alu_op_ile = 0xA5, midgard_alu_op_iball_eq = 0xA8, - midgard_alu_op_ball = 0xA9, + midgard_alu_op_iball_neq = 0xA9, midgard_alu_op_uball_lt = 0xAA, midgard_alu_op_uball_lte = 0xAB, midgard_alu_op_iball_lt = 0xAC, @@ -477,94 +477,6 @@ __attribute__((__packed__)) } midgard_texture_word; -/* Opcode name table */ - -static char *alu_opcode_names[256] = { - [midgard_alu_op_fadd] = "fadd", - [midgard_alu_op_fmul] = "fmul", - [midgard_alu_op_fmin] = "fmin", - [midgard_alu_op_fmax] = "fmax", - [midgard_alu_op_fmov] = "fmov", - [midgard_alu_op_froundeven] = "froundeven", - [midgard_alu_op_ftrunc] = "ftrunc", - [midgard_alu_op_ffloor] = "ffloor", - [midgard_alu_op_fceil] = "fceil", - [midgard_alu_op_ffma] = "ffma", - [midgard_alu_op_fdot3] = "fdot3", - [midgard_alu_op_fdot3r] = "fdot3r", - [midgard_alu_op_fdot4] = "fdot4", - [midgard_alu_op_freduce] = "freduce", - [midgard_alu_op_imin] = "imin", - [midgard_alu_op_umin] = "umin", - [midgard_alu_op_imax] = "imax", - [midgard_alu_op_umax] = "umax", - [midgard_alu_op_ishl] = "ishl", - [midgard_alu_op_iasr] = "iasr", - [midgard_alu_op_ilsr] = "ilsr", - [midgard_alu_op_iadd] = "iadd", - [midgard_alu_op_ishladd] = "ishladd", - [midgard_alu_op_isub] = "isub", - [midgard_alu_op_imul] = "imul", - [midgard_alu_op_imov] = "imov", - [midgard_alu_op_iabs] = "iabs", - [midgard_alu_op_iand] = "iand", - [midgard_alu_op_ior] = "ior", - [midgard_alu_op_inot] = "inot", - [midgard_alu_op_iandnot] = "iandnot", - [midgard_alu_op_ixor] = "ixor", - [midgard_alu_op_ilzcnt] = "ilzcnt", - [midgard_alu_op_ibitcount8] = "ibitcount8", - [midgard_alu_op_feq] = "feq", - [midgard_alu_op_fne] = "fne", - [midgard_alu_op_flt] = "flt", - [midgard_alu_op_fle] = "fle", - [midgard_alu_op_fball_eq] = "fball_eq", - [midgard_alu_op_fbany_neq] = "fbany_neq", - [midgard_alu_op_bball_eq] = "bball_eq", - [midgard_alu_op_fball_lt] = "fball_lt", - [midgard_alu_op_fball_lte] = "fball_lte", - [midgard_alu_op_bbany_neq] = "bbany_neq", - [midgard_alu_op_fbany_lt] = "fbany_lt", - [midgard_alu_op_fbany_lte] = "fbany_lte", - [midgard_alu_op_f2i] = "f2i", - [midgard_alu_op_f2u] = "f2u", - [midgard_alu_op_f2u8] = "f2u8", - [midgard_alu_op_ieq] = "ieq", - [midgard_alu_op_ine] = "ine", - [midgard_alu_op_ult] = "ult", - [midgard_alu_op_ule] = "ule", - [midgard_alu_op_ilt] = "ilt", - [midgard_alu_op_ile] = "ile", - [midgard_alu_op_iball_eq] = "iball_eq", - [midgard_alu_op_ball] = "ball", - [midgard_alu_op_uball_lt] = "uball_lt", - [midgard_alu_op_uball_lte] = "uball_lte", - [midgard_alu_op_iball_lt] = "iball_lt", - [midgard_alu_op_iball_lte] = "iball_lte", - [midgard_alu_op_iball_eq] = "iball_eq", - [midgard_alu_op_ibany_neq] = "ibany_neq", - [midgard_alu_op_ubany_lt] = "ubany_lt", - [midgard_alu_op_ubany_lte] = "ubany_lte", - [midgard_alu_op_ibany_lt] = "ibany_lt", - [midgard_alu_op_ibany_lte] = "ibany_lte", - [midgard_alu_op_i2f] = "i2f", - [midgard_alu_op_u2f] = "u2f", - [midgard_alu_op_icsel] = "icsel", - [midgard_alu_op_fcsel_i] = "fcsel_i", - [midgard_alu_op_fcsel] = "fcsel", - [midgard_alu_op_fround] = "fround", - [midgard_alu_op_fatan_pt2] = "fatan_pt2", - [midgard_alu_op_frcp] = "frcp", - [midgard_alu_op_frsqrt] = "frsqrt", - [midgard_alu_op_fsqrt] = "fsqrt", - [midgard_alu_op_fpow_pt1] = "fpow_pt1", - [midgard_alu_op_fexp2] = "fexp2", - [midgard_alu_op_flog2] = "flog2", - [midgard_alu_op_fsin] = "fsin", - [midgard_alu_op_fcos] = "fcos", - [midgard_alu_op_fatan2_pt1] = "fatan2_pt1" -}; - static char *load_store_opcode_names[256] = { [midgard_op_store_cubemap_coords] = "st_cubemap_coords", [midgard_op_load_attr_16] = "ld_attr_16", diff --git a/src/gallium/drivers/panfrost/midgard/midgard_compile.c b/src/gallium/drivers/panfrost/midgard/midgard_compile.c index 2c259ea525a..e5b1a1ec3fd 100644 --- a/src/gallium/drivers/panfrost/midgard/midgard_compile.c +++ b/src/gallium/drivers/panfrost/midgard/midgard_compile.c @@ -107,6 +107,12 @@ typedef struct midgard_branch { }; } midgard_branch; +static bool +midgard_is_branch_unit(unsigned unit) +{ + return (unit == ALU_ENAB_BRANCH) || (unit == ALU_ENAB_BR_COMPACT); +} + /* Generic in-memory data type repesenting a single logical instruction, rather * than a single instruction group. This is the preferred form for code gen. * Multiple midgard_insturctions will later be combined during scheduling, @@ -612,7 +618,7 @@ print_mir_instruction(midgard_instruction *ins) switch (ins->type) { case TAG_ALU_4: { midgard_alu_op op = ins->alu.op; - const char *name = alu_opcode_names[op]; + const char *name = alu_opcode_props[op].name; if (ins->unit) printf("%d.", ins->unit); @@ -946,7 +952,7 @@ effective_writemask(midgard_vector_alu *alu) /* Channel count is off-by-one to fit in two-bits (0 channel makes no * sense) */ - unsigned channel_count = GET_CHANNEL_COUNT(alu_opcode_props[alu->op]); + unsigned channel_count = GET_CHANNEL_COUNT(alu_opcode_props[alu->op].props); /* If there is a fixed channel count, construct the appropriate mask */ @@ -1240,7 +1246,7 @@ emit_alu(compiler_context *ctx, nir_alu_instr *instr) } /* Fetch unit, quirks, etc information */ - unsigned opcode_props = alu_opcode_props[op]; + unsigned opcode_props = alu_opcode_props[op].props; bool quirk_flipped_r24 = opcode_props & QUIRK_FLIPPED_R24; /* Initialise fields common between scalar/vector instructions */ @@ -2422,7 +2428,7 @@ schedule_bundle(compiler_context *ctx, midgard_block *block, midgard_instruction if (!unit) { int op = ains->alu.op; - int units = alu_opcode_props[op]; + int units = alu_opcode_props[op].props; /* TODO: Promotion of scalars to vectors */ int vector = ((!is_single_component_mask(ains->alu.mask)) || ((units & UNITS_SCALAR) == 0)) && (units & UNITS_ANY_VECTOR); @@ -2953,7 +2959,7 @@ embedded_to_inline_constant(compiler_context *ctx) case midgard_alu_op_fcsel: case midgard_alu_op_icsel: case midgard_alu_op_isub: - DBG("Missed non-commutative flip (%s)\n", alu_opcode_names[op]); + DBG("Missed non-commutative flip (%s)\n", alu_opcode_props[op].name); break; /* These ops are commutative and Just Flip */ -- 2.30.2