From 5500b1f2801cf7b0056cdbdec4d168bda58e36e0 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Thu, 30 Apr 2020 13:51:46 -0400 Subject: [PATCH] pan/mdg: Remove invert optimizations Unused since last commit. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/panfrost/Makefile.sources | 1 - src/panfrost/midgard/compiler.h | 14 - src/panfrost/midgard/meson.build | 1 - src/panfrost/midgard/midgard_compile.c | 21 -- src/panfrost/midgard/midgard_opt_invert.c | 420 ---------------------- src/panfrost/midgard/midgard_print.c | 2 +- src/panfrost/midgard/mir.c | 4 - 7 files changed, 1 insertion(+), 462 deletions(-) delete mode 100644 src/panfrost/midgard/midgard_opt_invert.c diff --git a/src/panfrost/Makefile.sources b/src/panfrost/Makefile.sources index 2760a5a65bd..ac7ef9daa99 100644 --- a/src/panfrost/Makefile.sources +++ b/src/panfrost/Makefile.sources @@ -54,7 +54,6 @@ midgard_FILES := \ midgard/midgard_opt_copy_prop.c \ midgard/midgard_opt_dce.c \ midgard/midgard_opt_float.c \ - midgard/midgard_opt_invert.c \ midgard/midgard_opt_perspective.c \ midgard/midgard-parse.h \ midgard/midgard_print.c \ diff --git a/src/panfrost/midgard/compiler.h b/src/panfrost/midgard/compiler.h index e0dd83bb9cf..67d56035a8b 100644 --- a/src/panfrost/midgard/compiler.h +++ b/src/panfrost/midgard/compiler.h @@ -130,12 +130,6 @@ typedef struct midgard_instruction { uint16_t mask; - /* For ALU ops only: set to true to invert (bitwise NOT) the - * destination of an integer-out op. Not implemented in hardware but - * allows more optimizations */ - - bool invert; - /* For accepting ALU ops - invert the nth source */ bool src_invert[MIR_SRC_COUNT]; @@ -659,14 +653,6 @@ bool midgard_opt_combine_projection(compiler_context *ctx, midgard_block *block) bool midgard_opt_varying_projection(compiler_context *ctx, midgard_block *block); bool midgard_opt_dead_code_eliminate(compiler_context *ctx, midgard_block *block); bool midgard_opt_dead_move_eliminate(compiler_context *ctx, midgard_block *block); - -void midgard_lower_invert(compiler_context *ctx, midgard_block *block); -bool midgard_opt_not_propagate(compiler_context *ctx, midgard_block *block); -bool midgard_opt_fuse_src_invert(compiler_context *ctx, midgard_block *block); -bool midgard_opt_fuse_dest_invert(compiler_context *ctx, midgard_block *block); -bool midgard_opt_csel_invert(compiler_context *ctx, midgard_block *block); bool midgard_opt_promote_fmov(compiler_context *ctx, midgard_block *block); -bool midgard_opt_drop_cmp_invert(compiler_context *ctx, midgard_block *block); -bool midgard_opt_invert_branch(compiler_context *ctx, midgard_block *block); #endif diff --git a/src/panfrost/midgard/meson.build b/src/panfrost/midgard/meson.build index b719af8bda9..65e1d096450 100644 --- a/src/panfrost/midgard/meson.build +++ b/src/panfrost/midgard/meson.build @@ -36,7 +36,6 @@ libpanfrost_midgard_files = files( 'mir_squeeze.c', 'midgard_opt_copy_prop.c', 'midgard_opt_dce.c', - 'midgard_opt_invert.c', 'midgard_opt_float.c', 'midgard_opt_perspective.c', 'midgard_errata_lod.c', diff --git a/src/panfrost/midgard/midgard_compile.c b/src/panfrost/midgard/midgard_compile.c index a47cc9bb791..d2575e5daca 100644 --- a/src/panfrost/midgard/midgard_compile.c +++ b/src/panfrost/midgard/midgard_compile.c @@ -2529,36 +2529,15 @@ midgard_compile_shader_nir(nir_shader *nir, panfrost_program *program, bool is_b progress |= midgard_opt_dead_code_eliminate(ctx, block); progress |= midgard_opt_combine_projection(ctx, block); progress |= midgard_opt_varying_projection(ctx, block); -#if 0 - progress |= midgard_opt_not_propagate(ctx, block); - progress |= midgard_opt_fuse_src_invert(ctx, block); - progress |= midgard_opt_fuse_dest_invert(ctx, block); - progress |= midgard_opt_csel_invert(ctx, block); - progress |= midgard_opt_drop_cmp_invert(ctx, block); - progress |= midgard_opt_invert_branch(ctx, block); -#endif } } while (progress); mir_foreach_block(ctx, _block) { midgard_block *block = (midgard_block *) _block; - //midgard_lower_invert(ctx, block); midgard_lower_derivatives(ctx, block); - } - - /* Nested control-flow can result in dead branches at the end of the - * block. This messes with our analysis and is just dead code, so cull - * them */ - mir_foreach_block(ctx, _block) { - midgard_block *block = (midgard_block *) _block; midgard_cull_dead_branch(ctx, block); } - /* Ensure we were lowered */ - mir_foreach_instr_global(ctx, ins) { - assert(!ins->invert); - } - if (ctx->stage == MESA_SHADER_FRAGMENT) mir_add_writeout_loops(ctx); diff --git a/src/panfrost/midgard/midgard_opt_invert.c b/src/panfrost/midgard/midgard_opt_invert.c deleted file mode 100644 index e9c1ef261a1..00000000000 --- a/src/panfrost/midgard/midgard_opt_invert.c +++ /dev/null @@ -1,420 +0,0 @@ -/* - * Copyright (C) 2019 Collabora, Ltd. - * - * Permission is hereby granted, free of charge, to any person obtaining a - * copy of this software and associated documentation files (the "Software"), - * to deal in the Software without restriction, including without limitation - * the rights to use, copy, modify, merge, publish, distribute, sublicense, - * and/or sell copies of the Software, and to permit persons to whom the - * Software is furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice (including the next - * paragraph) shall be included in all copies or substantial portions of the - * Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL - * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. - */ - -#include "compiler.h" -#include "midgard_ops.h" - -/* Lowers the invert field on instructions to a dedicated inot (inor) - * instruction instead, as invert is not always supported natively by the - * hardware */ - -void -midgard_lower_invert(compiler_context *ctx, midgard_block *block) -{ - mir_foreach_instr_in_block_safe(block, ins) { - if (ins->type != TAG_ALU_4) continue; - if (!ins->invert) continue; - - unsigned temp = make_compiler_temp(ctx); - - midgard_instruction not = { - .type = TAG_ALU_4, - .mask = ins->mask, - .src = { temp, ~0, ~0, ~0 }, - .swizzle = SWIZZLE_IDENTITY, - .dest = ins->dest, - .has_inline_constant = true, - .alu = { - .op = midgard_alu_op_inor, - /* TODO: i16 */ - .reg_mode = midgard_reg_mode_32, - .dest_override = midgard_dest_override_none, - .outmod = midgard_outmod_int_wrap - }, - }; - - ins->dest = temp; - ins->invert = false; - mir_insert_instruction_before(ctx, mir_next_op(ins), not); - } -} - -/* Propagate the .not up to the source */ - -bool -midgard_opt_not_propagate(compiler_context *ctx, midgard_block *block) -{ - bool progress = false; - - mir_foreach_instr_in_block_safe(block, ins) { - if (ins->type != TAG_ALU_4) continue; - if (ins->alu.op != midgard_alu_op_imov) continue; - if (!ins->invert) continue; - if (mir_nontrivial_source2_mod_simple(ins)) continue; - if (ins->src[1] & PAN_IS_REG) continue; - - /* Is it beneficial to propagate? */ - if (!mir_single_use(ctx, ins->src[1])) continue; - - /* We found an imov.not, propagate the invert back */ - - mir_foreach_instr_in_block_from_rev(block, v, mir_prev_op(ins)) { - if (v->dest != ins->src[1]) continue; - if (v->type != TAG_ALU_4) break; - - v->invert = !v->invert; - ins->invert = false; - progress |= true; - break; - } - } - - return progress; -} - -/* With that lowering out of the way, we can focus on more interesting - * optimizations. One easy one is fusing inverts into bitwise operations: - * - * ~iand = inand - * ~ior = inor - * ~ixor = inxor - */ - -static bool -mir_is_bitwise(midgard_instruction *ins) -{ - switch (ins->alu.op) { - case midgard_alu_op_iand: - case midgard_alu_op_ior: - case midgard_alu_op_ixor: - return true; - default: - return false; - } -} - -static bool -mir_is_inverted_bitwise(midgard_instruction *ins) -{ - switch (ins->alu.op) { - case midgard_alu_op_inand: - case midgard_alu_op_inor: - case midgard_alu_op_inxor: - return true; - default: - return false; - } -} - -static midgard_alu_op -mir_invert_op(midgard_alu_op op) -{ - switch (op) { - case midgard_alu_op_iand: - return midgard_alu_op_inand; - case midgard_alu_op_inand: - return midgard_alu_op_iand; - case midgard_alu_op_ior: - return midgard_alu_op_inor; - case midgard_alu_op_inor: - return midgard_alu_op_ior; - case midgard_alu_op_ixor: - return midgard_alu_op_inxor; - case midgard_alu_op_inxor: - return midgard_alu_op_ixor; - default: - unreachable("Op not invertible"); - } -} - -static midgard_alu_op -mir_demorgan_op(midgard_alu_op op) -{ - switch (op) { - case midgard_alu_op_iand: - return midgard_alu_op_inor; - case midgard_alu_op_ior: - return midgard_alu_op_inand; - default: - unreachable("Op not De Morgan-able"); - } -} - -static midgard_alu_op -mir_notright_op(midgard_alu_op op) -{ - switch (op) { - case midgard_alu_op_iand: - return midgard_alu_op_iandnot; - case midgard_alu_op_ior: - return midgard_alu_op_iornot; - default: - unreachable("Op not right able"); - } -} - -bool -midgard_opt_fuse_dest_invert(compiler_context *ctx, midgard_block *block) -{ - bool progress = false; - - mir_foreach_instr_in_block_safe(block, ins) { - /* Search for inverted bitwise */ - if (ins->type != TAG_ALU_4) continue; - if (!mir_is_bitwise(ins) && !mir_is_inverted_bitwise(ins)) continue; - if (!ins->invert) continue; - - ins->alu.op = mir_invert_op(ins->alu.op); - ins->invert = false; - progress |= true; - } - - return progress; -} - -/* Next up, we can fuse inverts into the sources of bitwise ops: - * - * ~a & b = b & ~a = iandnot(b, a) - * a & ~b = iandnot(a, b) - * ~a & ~b = ~(a | b) = inor(a, b) - * - * ~a | b = b | ~a = iornot(b, a) - * a | ~b = iornot(a, b) - * ~a | ~b = ~(a & b) = inand(a, b) - * - * ~a ^ b = ~(a ^ b) = inxor(a, b) - * a ^ ~b = ~(a ^ b) + inxor(a, b) - * ~a ^ ~b = a ^ b - * ~(a ^ b) = inxor(a, b) - */ - -static bool -mir_strip_inverted(compiler_context *ctx, unsigned node) -{ - if (node == SSA_FIXED_REGISTER(26)) - return false; - - /* Strips and returns the invert off a node */ - mir_foreach_instr_global(ctx, ins) { - if (ins->compact_branch) continue; - if (ins->dest != node) continue; - - bool status = ins->invert; - ins->invert = false; - return status; - } - - unreachable("Invalid node stripped"); -} - -static bool -is_ssa_or_constant(unsigned node) -{ - return !(node & PAN_IS_REG) || (node == SSA_FIXED_REGISTER(26)); -} - -bool -midgard_opt_fuse_src_invert(compiler_context *ctx, midgard_block *block) -{ - bool progress = false; - - mir_foreach_instr_in_block_safe(block, ins) { - /* Search for inverted bitwise */ - if (ins->type != TAG_ALU_4) continue; - if (!mir_is_bitwise(ins)) continue; - - if (!is_ssa_or_constant(ins->src[0])) continue; - if (!is_ssa_or_constant(ins->src[1])) continue; - if (!mir_single_use(ctx, ins->src[0])) continue; - if (!ins->has_inline_constant && !mir_single_use(ctx, ins->src[1])) continue; - - bool not_a = mir_strip_inverted(ctx, ins->src[0]); - bool not_b = - ins->has_inline_constant ? false : - mir_strip_inverted(ctx, ins->src[1]); - - /* Edge case: if src0 == src1, it'll've been stripped */ - if ((ins->src[0] == ins->src[1]) && !ins->has_inline_constant) - not_b = not_a; - - progress |= (not_a || not_b); - - /* No point */ - if (!(not_a || not_b)) continue; - - bool both = not_a && not_b; - bool left = not_a && !not_b; - bool right = !not_a && not_b; - - /* No-op, but we got to strip the inverts */ - if (both && ins->alu.op == midgard_alu_op_ixor) - continue; - - if (both) { - ins->alu.op = mir_demorgan_op(ins->alu.op); - } else if (right || (left && !ins->has_inline_constant)) { - /* Commute arguments */ - if (left) - mir_flip(ins); - - ins->alu.op = mir_notright_op(ins->alu.op); - } else if (left && ins->has_inline_constant) { - /* Some special transformations: - * - * ~A & c = ~(~(~A) | (~c)) = ~(A | ~c) = inor(A, ~c) - * ~A | c = ~(~(~A) & (~c)) = ~(A & ~c) = inand(A, ~c) - */ - - ins->alu.op = mir_demorgan_op(ins->alu.op); - ins->inline_constant = ~ins->inline_constant; - } - } - - return progress; -} - -/* Optimizes a .not away when used as the source of a conditional select: - * - * csel(a, b, c) = { b if a, c if !a } - * csel(!a, b, c) = { b if !a, c if !(!a) } = { c if a, b if !a } = csel(a, c, b) - * csel(!a, b, c) = csel(a, c, b) - */ - -bool -midgard_opt_csel_invert(compiler_context *ctx, midgard_block *block) -{ - bool progress = false; - - mir_foreach_instr_in_block_safe(block, ins) { - if (ins->type != TAG_ALU_4) continue; - if (!OP_IS_CSEL(ins->alu.op)) continue; - if (!is_ssa_or_constant(ins->src[2])) continue; - if (!mir_single_use(ctx, ins->src[2])) continue; - if (!mir_strip_inverted(ctx, ins->src[2])) continue; - - mir_flip(ins); - progress |= true; - } - - return progress; -} - - -static bool -mir_is_inverted(compiler_context *ctx, unsigned node) -{ - mir_foreach_instr_global(ctx, ins) { - if (ins->compact_branch) continue; - if (ins->dest != node) continue; - - return ins->invert; - } - - unreachable("Invalid node passed"); -} - - - -/* Optimizes comparisions which invert both arguments - * - * - * ieq(not(a), not(b)) = ieq(a, b) - * ine(not(a), not(b)) = ine(a, b) - * - * This does apply for ilt and ile if we flip the argument order: - * Proofs below provided by Alyssa Rosenzweig - * - * not(x) = −(x+1) - * - * ( not(A) <= not(B) ) <=> ( −(A+1) <= −(B+1) ) - * <=> ( A+1 >= B+1) - * <=> ( B <= A ) - * - * On unsigned comparisons (ult / ule) we can perform the same optimization - * with the additional restriction that the source registers must - * have the same size. - * - * TODO: We may not need them to be of the same size, if we can - * prove that they are the same after sext/zext - * - * not(x) = 2n−x−1 - * - * ( not(A) <= not(B) ) <=> ( 2n−A−1 <= 2n−B−1 ) - * <=> ( −A <= −B ) - * <=> ( B <= A ) - */ -bool -midgard_opt_drop_cmp_invert(compiler_context *ctx, midgard_block *block) -{ - - bool progress = false; - - mir_foreach_instr_in_block_safe(block, ins) { - if (ins->type != TAG_ALU_4) continue; - if (!OP_IS_INTEGER_CMP(ins->alu.op)) continue; - - if ((ins->src[0] & PAN_IS_REG) || (ins->src[1] & PAN_IS_REG)) continue; - if (!mir_single_use(ctx, ins->src[0]) || !mir_single_use(ctx, ins->src[1])) continue; - - bool a_inverted = mir_is_inverted(ctx, ins->src[0]); - bool b_inverted = mir_is_inverted(ctx, ins->src[1]); - - if (!a_inverted || !b_inverted) continue; - if (OP_IS_UNSIGNED_CMP(ins->alu.op) && mir_srcsize(ins, 0) != mir_srcsize(ins, 1)) continue; - - - mir_strip_inverted(ctx, ins->src[0]); - mir_strip_inverted(ctx, ins->src[1]); - - if (ins->alu.op != midgard_alu_op_ieq && ins->alu.op != midgard_alu_op_ine) - mir_flip(ins); - - progress |= true; - } - - return progress; -} - -/* Optimizes branches with inverted arguments by inverting the - * branch condition instead of the argument condition. - */ -bool -midgard_opt_invert_branch(compiler_context *ctx, midgard_block *block) -{ - bool progress = false; - - mir_foreach_instr_in_block_safe(block, ins) { - if (ins->type != TAG_ALU_4) continue; - if (!midgard_is_branch_unit(ins->unit)) continue; - if (!ins->branch.conditional) continue; - if (ins->src[0] & PAN_IS_REG) continue; - - if (mir_strip_inverted(ctx, ins->src[0])) { - ins->branch.invert_conditional = !ins->branch.invert_conditional; - - progress |= true; - } - } - - return progress; -} diff --git a/src/panfrost/midgard/midgard_print.c b/src/panfrost/midgard/midgard_print.c index 1b1ff6a145f..fa7792e33e1 100644 --- a/src/panfrost/midgard/midgard_print.c +++ b/src/panfrost/midgard/midgard_print.c @@ -342,7 +342,7 @@ mir_print_instruction(midgard_instruction *ins) assert(0); } - if (ins->invert || (ins->compact_branch && ins->branch.invert_conditional)) + if (ins->compact_branch && ins->branch.invert_conditional) printf(".not"); printf(" "); diff --git a/src/panfrost/midgard/mir.c b/src/panfrost/midgard/mir.c index 772b6956de3..7dc357e62ac 100644 --- a/src/panfrost/midgard/mir.c +++ b/src/panfrost/midgard/mir.c @@ -166,10 +166,6 @@ mir_nontrivial_outmod(midgard_instruction *ins) bool is_int = midgard_is_integer_op(ins->alu.op); unsigned mod = ins->alu.outmod; - /* Pseudo-outmod */ - if (ins->invert) - return true; - /* Type conversion is a sort of outmod */ if (ins->alu.dest_override != midgard_dest_override_none) return true; -- 2.30.2