From 420b58363b3924b5f17aacc043f186085c4ce6ef Mon Sep 17 00:00:00 2001 From: Alexander Root Date: Mon, 22 Aug 2022 15:53:51 -0400 Subject: [PATCH 1/9] implement widen_right_ ops --- src/CodeGen_ARM.cpp | 28 ++++ src/FindIntrinsics.cpp | 212 ++++++++++++++++++++++++- src/FindIntrinsics.h | 3 + src/HexagonOptimize.cpp | 59 ++++--- src/IR.cpp | 3 + src/IR.h | 9 ++ src/IRMatch.h | 80 ++++++++-- src/IROperator.cpp | 34 ++++ src/IROperator.h | 7 + src/runtime/hvx_128.ll | 12 -- test/correctness/intrinsics.cpp | 2 +- test/correctness/simd_op_check_hvx.cpp | 2 +- 12 files changed, 391 insertions(+), 60 deletions(-) diff --git a/src/CodeGen_ARM.cpp b/src/CodeGen_ARM.cpp index 06acff932f9e..58ede08568ac 100644 --- a/src/CodeGen_ARM.cpp +++ b/src/CodeGen_ARM.cpp @@ -1196,6 +1196,33 @@ void CodeGen_ARM::visit(const Call *op) { } } + auto rewrite = IRMatcher::rewriter(op, op->type); + IRMatcher::Wild<0> x; + IRMatcher::Wild<1> y; + const int bits = op->type.bits(); + const int lanes = op->type.lanes(); + auto y_2x = is_int(y, bits / 2, lanes * 2) || is_uint(y, bits / 2, lanes * 2); + + // Look for (S | U)ADALP patterns. + // This would be much easier to do if we matched on reduction factors of + // VectorReduce nodes instead of the output lanes. + if (// TODO: is there a way to not just undo what FindIntrinsics does here? + rewrite( + widen_right_add(x, h_add(y, lanes)), + x + h_add(cast(op->type.with_lanes(lanes * 2), y), lanes), + y_2x) || + + rewrite( + widen_right_add(x, h_add(y, lanes)), + x + h_add(cast(op->type.with_lanes(lanes * 2), h_add(y, lanes * 2)), lanes), + // y must be even to do this split. + has_even_lanes(y)) || + + false) { + value = codegen(rewrite.result); + return; + } + if (target.has_feature(Target::ARMFp16)) { auto it = float16_transcendental_remapping.find(op->name); if (it != float16_transcendental_remapping.end()) { @@ -1331,6 +1358,7 @@ void CodeGen_ARM::codegen_vector_reduce(const VectorReduce *op, const Expr &init if (narrow.defined()) { if (init.defined() && target.bits == 32) { // On 32-bit, we have an intrinsic for widening add-accumulate. + // TODO: this could be written as a pattern with widen_right_add (#6951). intrin = "pairwise_widening_add_accumulate"; intrin_args = {accumulator, narrow}; accumulator = Expr(); diff --git a/src/FindIntrinsics.cpp b/src/FindIntrinsics.cpp index 0a33b85822aa..f1b36ea6a710 100644 --- a/src/FindIntrinsics.cpp +++ b/src/FindIntrinsics.cpp @@ -74,7 +74,7 @@ bool is_safe_for_add(const Expr &e, int max_depth) { } else if (cast->type.bits() == cast->value.type().bits()) { return is_safe_for_add(cast->value, max_depth); } - } else if (Call::as_intrinsic(e, {Call::widening_add, Call::widening_sub})) { + } else if (Call::as_intrinsic(e, {Call::widening_add, Call::widening_sub, Call::widen_right_add, Call::widen_right_sub})) { return true; } return false; @@ -115,6 +115,7 @@ Expr to_rounding_shift(const Call *c) { Expr a = c->args[0]; Expr b = c->args[1]; + // Helper to make the appropriate shift. auto rounding_shift = [&](const Expr &a, const Expr &b) { if (c->is_intrinsic(Call::shift_right)) { @@ -131,20 +132,47 @@ Expr to_rounding_shift(const Call *c) { } Expr round; if (c->is_intrinsic(Call::shift_right)) { - round = simplify((make_one(round_type) << max(cast(b.type().with_bits(round_type.bits()), b), 0)) / 2); + round = (make_one(round_type) << max(cast(b.type().with_bits(round_type.bits()), b), 0)) / 2; } else { - round = simplify((make_one(round_type) >> min(cast(b.type().with_bits(round_type.bits()), b), 0)) / 2); + round = (make_one(round_type) >> min(cast(b.type().with_bits(round_type.bits()), b), 0)) / 2; } + // Input expressions are simplified before running find_intrinsics, but b + // has been lifted here so we need to lower_intrinsics before simplifying + // and re-lifting. Should we move this code into the FindIntrinsics class + // to make it easier to lift round? + round = lower_intrinsics(round); + round = simplify(round); + round = find_intrinsics(round); // We can always handle widening adds. if (const Call *add = Call::as_intrinsic(a, {Call::widening_add})) { - if (can_prove(lower_intrinsics(add->args[0]) == round)) { + if (can_prove(lower_intrinsics(add->args[0] == round))) { return rounding_shift(cast(add->type, add->args[1]), b); - } else if (can_prove(lower_intrinsics(add->args[1]) == round)) { + } else if (can_prove(lower_intrinsics(add->args[1] == round))) { return rounding_shift(cast(add->type, add->args[0]), b); } } + if (const Call *add = Call::as_intrinsic(a, {Call::widen_right_add})) { + if (can_prove(lower_intrinsics(add->args[1] == round))) { + return rounding_shift(cast(add->type, add->args[0]), b); + } + } + // Also need to handle the annoying case of a reinterpret wrapping a widen_right_add + // TODO: this pattern makes me want to change the semantics of this op. + if (const Reinterpret *reinterp = a.as()) { + if (reinterp->type.bits() == reinterp->value.type().bits()) { + if (const Call *add = Call::as_intrinsic(reinterp->value, {Call::widen_right_add})) { + if (can_prove(lower_intrinsics(add->args[1] == round))) { + // We expect the first operand to be a reinterpet. + const Reinterpret *reinterp_a = add->args[0].as(); + internal_assert(reinterp_a) << "Failed: " << add->args[0] << "\n"; + return rounding_shift(reinterp_a->value, b); + } + } + } + } + // If it wasn't a widening or saturating add, we might still // be able to safely accept the rounding. Expr a_less_round = find_and_subtract(a, round); @@ -199,6 +227,52 @@ class FindIntrinsics : public IRMutator { } } + if (op->type.is_int_or_uint() && op->type.bits() > 8) { + // Look for widen_right_add intrinsics. + // Yes we do an duplicate code, but we want to check the op->type.code() first, + // and the opposite as well. + for (halide_type_code_t code : {op->type.code(), halide_type_uint, halide_type_int}) { + Type narrow = op->type.narrow().with_code(code); + Expr narrow_a = lossless_cast(narrow, a); + Expr narrow_b = lossless_cast(narrow, b); + + // This case should have been handled by the above check for widening_add. + internal_assert(!(narrow_a.defined() && narrow_b.defined())) + << "find_intrinsics failed to find a widening_add: " << a << " + " << b << "\n"; + + if (narrow_a.defined()) { + Expr result; + if (b.type().code() != narrow_a.type().code()) { + // Need to do a safe reinterpret. + Type t = b.type().with_code(code); + result = widen_right_add(reinterpret(t, b), narrow_a); + internal_assert(result.type() != op->type); + result = reinterpret(op->type, result); + } else { + result = widen_right_add(b, narrow_a); + } + internal_assert(result.type() == op->type); + return result; + } else if (narrow_b.defined()) { + Expr result; + if (a.type().code() != narrow_b.type().code()) { + // Need to do a safe reinterpret. + Type t = a.type().with_code(code); + result = widen_right_add(reinterpret(t, a), narrow_b); + internal_assert(result.type() != op->type); + result = reinterpret(op->type, result); + } else { + result = widen_right_add(a, narrow_b); + } + internal_assert(result.type() == op->type); + return result; + } + } + } + + // TODO: there can be widen_right_add + widen_right_add simplification rules. + // i.e. widen_right_add(a, b) + widen_right_add(c, d) = (a + c) + widening_add(b, d) + if (a.same_as(op->a) && b.same_as(op->b)) { return op; } else { @@ -240,6 +314,34 @@ class FindIntrinsics : public IRMutator { return Add::make(a, negative_b); } + + // Run after the lossless_negate check, because we want that to turn into an widen_right_add if relevant. + if (op->type.is_int_or_uint() && op->type.bits() > 8) { + // Look for widen_right_sub intrinsics. + // Yes we do an duplicate code, but we want to check the op->type.code() first, + // and the opposite as well. + for (halide_type_code_t code : {op->type.code(), halide_type_uint, halide_type_int}) { + Type narrow = op->type.narrow().with_code(code); + Expr narrow_b = lossless_cast(narrow, b); + + if (narrow_b.defined()) { + Expr result; + if (a.type().code() != narrow_b.type().code()) { + // Need to do a safe reinterpret. + Type t = a.type().with_code(code); + result = widen_right_sub(reinterpret(t, a), narrow_b); + internal_assert(result.type() != op->type); + result = reinterpret(op->type, result); + } else { + result = widen_right_sub(a, narrow_b); + } + internal_assert(result.type() == op->type); + return result; + } + } + } + + if (a.same_as(op->a) && b.same_as(op->b)) { return op; } else { @@ -292,6 +394,49 @@ class FindIntrinsics : public IRMutator { return mutate(result); } + if (op->type.is_int_or_uint() && op->type.bits() > 8) { + // Look for widen_right_mul intrinsics. + // Yes we do an duplicate code, but we want to check the op->type.code() first, + // and the opposite as well. + for (halide_type_code_t code : {op->type.code(), halide_type_uint, halide_type_int}) { + Type narrow = op->type.narrow().with_code(code); + Expr narrow_a = lossless_cast(narrow, a); + Expr narrow_b = lossless_cast(narrow, b); + + // This case should have been handled by the above check for widening_mul. + internal_assert(!(narrow_a.defined() && narrow_b.defined())) + << "find_intrinsics failed to find a widening_mul: " << a << " + " << b << "\n"; + + if (narrow_a.defined()) { + Expr result; + if (b.type().code() != narrow_a.type().code()) { + // Need to do a safe reinterpret. + Type t = b.type().with_code(code); + result = widen_right_mul(reinterpret(t, b), narrow_a); + internal_assert(result.type() != op->type); + result = reinterpret(op->type, result); + } else { + result = widen_right_mul(b, narrow_a); + } + internal_assert(result.type() == op->type); + return result; + } else if (narrow_b.defined()) { + Expr result; + if (a.type().code() != narrow_b.type().code()) { + // Need to do a safe reinterpret. + Type t = a.type().with_code(code); + result = widen_right_mul(reinterpret(t, a), narrow_b); + internal_assert(result.type() != op->type); + result = reinterpret(op->type, result); + } else { + result = widen_right_mul(a, narrow_b); + } + internal_assert(result.type() == op->type); + return result; + } + } + } + if (a.same_as(op->a) && b.same_as(op->b)) { return op; } else { @@ -593,7 +738,39 @@ class FindIntrinsics : public IRMutator { Type opposite_type = op->type.is_int() ? op->type.with_code(halide_type_uint) : op->type.with_code(halide_type_int); const auto is_x_wider_opposite_int = (op->type.is_int() && is_uint(x, 2 * bits)) || (op->type.is_uint() && is_int(x, 2 * bits)); + if ( + // Simplify extending patterns. + // (x + widen(y)) + widen(z) = x + widening_add(y, z). + rewrite(widen_right_add(widen_right_add(x, y), z), + x + widening_add(y, z), + // We only care about integers, this should be trivially true. + is_x_same_int_or_uint) || + + // (x - widen(y)) - widen(z) = x - widening_add(y, z). + rewrite(widen_right_sub(widen_right_sub(x, y), z), + x - widening_add(y, z), + // We only care about integers, this should be trivially true. + is_x_same_int_or_uint) || + + // (x + widen(y)) - widen(z) = x + cast(t, widening_sub(y, z)) + // cast (reinterpret) is needed only for uints. + rewrite(widen_right_sub(widen_right_add(x, y), z), + x + widening_sub(y, z), + is_x_same_int) || + rewrite(widen_right_sub(widen_right_add(x, y), z), + x + cast(op->type, widening_sub(y, z)), + is_x_same_uint) || + + // (x - widen(y)) + widen(z) = x + cast(t, widening_sub(z, y)) + // cast (reinterpret) is needed only for uints. + rewrite(widen_right_add(widen_right_sub(x, y), z), + x + widening_sub(z, y), + is_x_same_int) || + rewrite(widen_right_add(widen_right_sub(x, y), z), + x + cast(op->type, widening_sub(z, y)), + is_x_same_uint) || + // Saturating patterns. rewrite(saturating_cast(op->type, widening_add(x, y)), saturating_add(x, y), @@ -679,6 +856,7 @@ class FindIntrinsics : public IRMutator { } } } + // TODO: do we want versions of widen_right_add here? if (op->is_intrinsic(Call::shift_right) || op->is_intrinsic(Call::shift_left)) { // Try to turn this into a widening shift. @@ -885,6 +1063,19 @@ Expr find_intrinsics(const Expr &e) { return expr; } +Expr lower_widen_right_add(const Expr &a, const Expr &b) { + return a + widen(b); +} + +Expr lower_widen_right_mul(const Expr &a, const Expr &b) { + return a * widen(b); +} + + +Expr lower_widen_right_sub(const Expr &a, const Expr &b) { + return a - widen(b); +} + Expr lower_widening_add(const Expr &a, const Expr &b) { return widen(a) + widen(b); } @@ -1100,7 +1291,16 @@ Expr lower_rounding_mul_shift_right(const Expr &a, const Expr &b, const Expr &q) } Expr lower_intrinsic(const Call *op) { - if (op->is_intrinsic(Call::widening_add)) { + if (op->is_intrinsic(Call::widen_right_add)) { + internal_assert(op->args.size() == 2); + return lower_widen_right_add(op->args[0], op->args[1]); + } else if (op->is_intrinsic(Call::widen_right_mul)) { + internal_assert(op->args.size() == 2); + return lower_widen_right_mul(op->args[0], op->args[1]); + } else if (op->is_intrinsic(Call::widen_right_sub)) { + internal_assert(op->args.size() == 2); + return lower_widen_right_sub(op->args[0], op->args[1]); + } else if (op->is_intrinsic(Call::widening_add)) { internal_assert(op->args.size() == 2); return lower_widening_add(op->args[0], op->args[1]); } else if (op->is_intrinsic(Call::widening_mul)) { diff --git a/src/FindIntrinsics.h b/src/FindIntrinsics.h index 07e639117252..f8ddaf171bc3 100644 --- a/src/FindIntrinsics.h +++ b/src/FindIntrinsics.h @@ -11,6 +11,9 @@ namespace Halide { namespace Internal { /** Implement intrinsics with non-intrinsic using equivalents. */ +Expr lower_widen_right_add(const Expr &a, const Expr &b); +Expr lower_widen_right_mul(const Expr &a, const Expr &b); +Expr lower_widen_right_sub(const Expr &a, const Expr &b); Expr lower_widening_add(const Expr &a, const Expr &b); Expr lower_widening_mul(const Expr &a, const Expr &b); Expr lower_widening_sub(const Expr &a, const Expr &b); diff --git a/src/HexagonOptimize.cpp b/src/HexagonOptimize.cpp index 1cdc525398df..2e6b1ee23adc 100644 --- a/src/HexagonOptimize.cpp +++ b/src/HexagonOptimize.cpp @@ -137,6 +137,8 @@ Expr as_mul(const Expr &a) { Expr b = make_one(s->type) << cast(UInt(s->type.bits()), (int)*log2_b); return simplify(Mul::make(cast(s->type, s->args[0]), b)); } + } else if (const Call *wm = Call::as_intrinsic(a, {Call::widen_right_mul})) { + return simplify(Mul::make(wm->args[0], cast(wm->type, wm->args[1]))); } return Expr(); } @@ -426,6 +428,11 @@ int find_mpy_ops(const Expr &op, Type a_ty, Type b_ty, int max_mpy_count, mpy_count += find_mpy_ops(cast(op.type(), add->args[0]), a_ty, b_ty, max_mpy_count, mpys, rest); mpy_count += find_mpy_ops(cast(op.type(), add->args[1]), a_ty, b_ty, max_mpy_count, mpys, rest); return mpy_count; + } else if (const Call *wadd = Call::as_intrinsic(op, {Call::widen_right_add})) { + int mpy_count = 0; + mpy_count += find_mpy_ops(wadd->args[0], a_ty, b_ty, max_mpy_count, mpys, rest); + mpy_count += find_mpy_ops(cast(op.type(), wadd->args[1]), a_ty, b_ty, max_mpy_count, mpys, rest); + return mpy_count; } // Attempt to pretend this op is multiplied by 1. @@ -451,37 +458,7 @@ class OptimizePatterns : public IRMutator { Scope bounds; const Target ⌖ - Expr visit(const Mul *op) override { - static const vector scalar_muls = { - // Non-widening scalar multiplication. - {"halide.hexagon.mul.vh.b", wild_i16x * wild_i16, Pattern::NarrowOp1}, - {"halide.hexagon.mul.vw.h", wild_i32x * wild_i32, Pattern::NarrowOp1}, - // TODO: There's also mul.vw.b. We currently generate mul.vw.h - // instead. I'm not sure mul.vw.b is faster, it might even be - // slower due to the extra step in broadcasting the scalar up to - // 32 bits. - }; - - static const vector muls = { - // One operand widening multiplication. - {"halide.hexagon.mul.vw.vh", wild_i32x * wild_i32x, Pattern::ReinterleaveOp0 | Pattern::NarrowOp1}, - {"halide.hexagon.mul.vw.vuh", wild_i32x * wild_i32x, Pattern::ReinterleaveOp0 | Pattern::NarrowUnsignedOp1}, - {"halide.hexagon.mul.vuw.vuh", wild_u32x * wild_u32x, Pattern::ReinterleaveOp0 | Pattern::NarrowUnsignedOp1}, - }; - - if (op->type.is_vector()) { - Expr new_expr = apply_commutative_patterns(op, scalar_muls, target, this); - if (!new_expr.same_as(op)) { - return new_expr; - } - - new_expr = apply_commutative_patterns(op, muls, target, this); - if (!new_expr.same_as(op)) { - return new_expr; - } - } - return IRMutator::visit(op); - } + // Interesting muls are handled as widen_right_mul(). // We'll try to sort the mpys based my mpys.first. // But, for this all the mpy.first exprs should either be @@ -884,6 +861,14 @@ class OptimizePatterns : public IRMutator { return mpyadds; } } + if (op->is_intrinsic(Call::widen_right_add)) { + Expr mpyadds = find_mpyadds(Add::make(op->args[0], cast(op->type, op->args[1]))); + if (mpyadds.defined()) { + return mpyadds; + } + } + + // TODO: do widen_right_adds // These intrinsics should get the default lowering, and we need to recursively mutate the // result. We don't want to let these fall through to CodeGen_Hexagon and CodeGen_LLVM, @@ -900,6 +885,18 @@ class OptimizePatterns : public IRMutator { } static const vector calls = { + // Non-widening scalar multiplication. + {"halide.hexagon.mul.vh.b", widen_right_mul(wild_i16x, wild_i8)}, + {"halide.hexagon.mul.vw.h", widen_right_mul(wild_i32x, wild_i16)}, + // TODO: There's also mul.vw.b. We currently generate mul.vw.h + // instead. I'm not sure mul.vw.b is faster, it might even be + // slower due to the extra step in broadcasting the scalar up to + // 32 bits. + + // One operand widening multiplication. + {"halide.hexagon.mul.vw.vh", widen_right_mul(wild_i32x, wild_i16x), Pattern::ReinterleaveOp0}, + {"halide.hexagon.mul.vw.vuh", widen_right_mul(wild_u32x, wild_u16x), Pattern::ReinterleaveOp0}, + // Saturating narrowing casts with rounding {"halide.hexagon.trunc_satub_rnd.vh", u8_sat(rounding_shift_right(wild_i16x, 8)), Pattern::DeinterleaveOp0}, {"halide.hexagon.trunc_satb_rnd.vh", i8_sat(rounding_shift_right(wild_i16x, 8)), Pattern::DeinterleaveOp0}, diff --git a/src/IR.cpp b/src/IR.cpp index 105472ab68d6..e2f54318214a 100644 --- a/src/IR.cpp +++ b/src/IR.cpp @@ -669,6 +669,9 @@ const char *const intrinsic_op_names[] = { "undef", "unreachable", "unsafe_promise_clamped", + "widen_right_add", + "widen_right_mul", + "widen_right_sub", "widening_add", "widening_mul", "widening_shift_left", diff --git a/src/IR.h b/src/IR.h index ff92f38d4107..0da5ffa1aaa6 100644 --- a/src/IR.h +++ b/src/IR.h @@ -583,6 +583,15 @@ struct Call : public ExprNode { undef, unreachable, unsafe_promise_clamped, + + // One-sided variants of widening_add, widening_mul, and widening_sub. + // arg[0] + widen(arg[1]) + widen_right_add, + // arg[0] * widen(arg[1]) + widen_right_mul, + // arg[0] - widen(arg[1]) + widen_right_sub, + widening_add, widening_mul, widening_shift_left, diff --git a/src/IRMatch.h b/src/IRMatch.h index 0de1c21cee44..06d35ba8b4b0 100644 --- a/src/IRMatch.h +++ b/src/IRMatch.h @@ -1429,6 +1429,12 @@ struct Intrin { Expr arg1 = std::get(args).make(state, type_hint); if (intrin == Call::absd) { return absd(arg0, arg1); + } else if (intrin == Call::widen_right_add) { + return widen_right_add(arg0, arg1); + } else if (intrin == Call::widen_right_mul) { + return widen_right_mul(arg0, arg1); + } else if (intrin == Call::widen_right_sub) { + return widen_right_sub(arg0, arg1); } else if (intrin == Call::widening_add) { return widening_add(arg0, arg1); } else if (intrin == Call::widening_sub) { @@ -1529,6 +1535,19 @@ HALIDE_ALWAYS_INLINE auto intrin(Call::IntrinsicOp intrinsic_op, Args... args) n return {intrinsic_op, pattern_arg(args)...}; } +template +auto widen_right_add(A &&a, B &&b) noexcept -> Intrin { + return {Call::widen_right_add, pattern_arg(a), pattern_arg(b)}; +} +template +auto widen_right_mul(A &&a, B &&b) noexcept -> Intrin { + return {Call::widen_right_mul, pattern_arg(a), pattern_arg(b)}; +} +template +auto widen_right_sub(A &&a, B &&b) noexcept -> Intrin { + return {Call::widen_right_sub, pattern_arg(a), pattern_arg(b)}; +} + template auto widening_add(A &&a, B &&b) noexcept -> Intrin { return {Call::widening_add, pattern_arg(a), pattern_arg(b)}; @@ -1859,7 +1878,7 @@ struct VectorReduceOp { A a; B lanes; - constexpr static uint32_t binds = bindings::mask; + constexpr static uint32_t binds = bindings::mask | bindings::mask; constexpr static IRNodeType min_node_type = IRNodeType::VectorReduce; constexpr static IRNodeType max_node_type = IRNodeType::VectorReduce; @@ -2324,7 +2343,7 @@ template struct IsInt { struct pattern_tag {}; A a; - int bits; + int bits, lanes; constexpr static uint32_t binds = bindings::mask; @@ -2339,7 +2358,7 @@ struct IsInt { void make_folded_const(halide_scalar_value_t &val, halide_type_t &ty, MatcherState &state) const { // a is almost certainly a very simple pattern (e.g. a wild), so just inline the make method. Type t = a.make(state, {}).type(); - val.u.u64 = t.is_int() && (bits == 0 || t.bits() == bits); + val.u.u64 = t.is_int() && (bits == 0 || t.bits() == bits) && (lanes == 0 || t.lanes() == lanes); ty.code = halide_type_uint; ty.bits = 1; ty.lanes = t.lanes(); @@ -2347,9 +2366,9 @@ struct IsInt { }; template -HALIDE_ALWAYS_INLINE auto is_int(A &&a, int bits = 0) noexcept -> IsInt { +HALIDE_ALWAYS_INLINE auto is_int(A &&a, int bits = 0, int lanes = 0) noexcept -> IsInt { assert_is_lvalue_if_expr(); - return {pattern_arg(a), bits}; + return {pattern_arg(a), bits, lanes}; } template @@ -2358,6 +2377,9 @@ std::ostream &operator<<(std::ostream &s, const IsInt &op) { if (op.bits > 0) { s << ", " << op.bits; } + if (op.lanes > 0) { + s << ", " << op.lanes; + } s << ")"; return s; } @@ -2366,7 +2388,7 @@ template struct IsUInt { struct pattern_tag {}; A a; - int bits; + int bits, lanes; constexpr static uint32_t binds = bindings::mask; @@ -2381,7 +2403,7 @@ struct IsUInt { void make_folded_const(halide_scalar_value_t &val, halide_type_t &ty, MatcherState &state) const { // a is almost certainly a very simple pattern (e.g. a wild), so just inline the make method. Type t = a.make(state, {}).type(); - val.u.u64 = t.is_uint() && (bits == 0 || t.bits() == bits); + val.u.u64 = t.is_uint() && (bits == 0 || t.bits() == bits) && (lanes == 0 || t.lanes() == lanes); ty.code = halide_type_uint; ty.bits = 1; ty.lanes = t.lanes(); @@ -2389,9 +2411,9 @@ struct IsUInt { }; template -HALIDE_ALWAYS_INLINE auto is_uint(A &&a, int bits = 0) noexcept -> IsUInt { +HALIDE_ALWAYS_INLINE auto is_uint(A &&a, int bits = 0, int lanes = 0) noexcept -> IsUInt { assert_is_lvalue_if_expr(); - return {pattern_arg(a), bits}; + return {pattern_arg(a), bits, lanes}; } template @@ -2400,6 +2422,9 @@ std::ostream &operator<<(std::ostream &s, const IsUInt &op) { if (op.bits > 0) { s << ", " << op.bits; } + if (op.lanes > 0) { + s << ", " << op.lanes; + } s << ")"; return s; } @@ -2525,6 +2550,43 @@ std::ostream &operator<<(std::ostream &s, const IsMinValue &op) { return s; } +template +struct HasEvenLanes { + struct pattern_tag {}; + A a; + + constexpr static uint32_t binds = bindings::mask; + + // This rule is a boolean-valued predicate. Bools have type UIntImm. + constexpr static IRNodeType min_node_type = IRNodeType::UIntImm; + constexpr static IRNodeType max_node_type = IRNodeType::UIntImm; + constexpr static bool canonical = true; + + constexpr static bool foldable = true; + + HALIDE_ALWAYS_INLINE + void make_folded_const(halide_scalar_value_t &val, halide_type_t &ty, MatcherState &state) const { + // a is almost certainly a very simple pattern (e.g. a wild), so just inline the make method. + Type t = a.make(state, {}).type(); + val.u.u64 = (t.lanes() % 2 == 0); + ty.code = halide_type_uint; + ty.bits = 1; + ty.lanes = t.lanes(); + } +}; + +template +HALIDE_ALWAYS_INLINE auto has_even_lanes(A &&a) noexcept -> HasEvenLanes { + assert_is_lvalue_if_expr(); + return {pattern_arg(a)}; +} + +template +std::ostream &operator<<(std::ostream &s, const HasEvenLanes &op) { + s << "has_even_lanes(" << op.a << ")"; + return s; +} + // Verify properties of each rewrite rule. Currently just fuzz tests them. template &cache_key_values) args, Internal::Call::PureIntrinsic); } +Expr widen_right_add(Expr a, Expr b) { + user_assert(a.defined() && b.defined()) << "widen_right_add of undefined Expr\n" << a << ", " << b << "\n"; + user_assert(a.type().is_int_or_uint() && b.type().is_int_or_uint()) + << "widen_right_add only defined for integer types, received:\n " << a << "\n " << b << "\n"; + user_assert(b.type().bits() <= 32) << "widen_right_add of large Expr\n" << a << ", " << b << "\n"; + match_lanes(a, b); + Type wide_type = b.type().widen(); + user_assert(wide_type == a.type()) << "widen_right_add type mismatch\n " << a << "\n " << b << "\n"; + return Call::make(wide_type, Call::widen_right_add, {std::move(a), std::move(b)}, Call::PureIntrinsic); +} + +Expr widen_right_mul(Expr a, Expr b) { + user_assert(a.defined() && b.defined()) << "widen_right_mul of undefined Expr\n" << a << ", " << b << "\n"; + user_assert(a.type().is_int_or_uint() && b.type().is_int_or_uint()) + << "widen_right_mul only defined for integer types, received:\n " << a << "\n " << b << "\n"; + user_assert(b.type().bits() <= 32) << "widen_right_mul of large Expr\n" << a << ", " << b << "\n"; + match_lanes(a, b); + Type wide_type = b.type().widen(); + user_assert(wide_type == a.type()) << "widen_right_mul type mismatch\n " << a << "\n " << b << "\n"; + return Call::make(wide_type, Call::widen_right_mul, {std::move(a), std::move(b)}, Call::PureIntrinsic); +} + +Expr widen_right_sub(Expr a, Expr b) { + user_assert(a.defined() && b.defined()) << "widen_right_sub of undefined Expr\n" << a << ", " << b << "\n"; + user_assert(a.type().is_int_or_uint() && b.type().is_int_or_uint()) + << "widen_right_sub only defined for integer types, received:\n " << a << "\n " << b << "\n"; + user_assert(b.type().bits() <= 32) << "widen_right_sub of large Expr\n" << a << ", " << b << "\n"; + match_lanes(a, b); + Type wide_type = b.type().widen(); + user_assert(wide_type == a.type()) << "widen_right_sub type mismatch\n" << a << ", " << b << "\n"; + return Call::make(wide_type, Call::widen_right_sub, {std::move(a), std::move(b)}, Call::PureIntrinsic); +} + + Expr widening_add(Expr a, Expr b) { user_assert(a.defined() && b.defined()) << "widening_add of undefined Expr\n"; match_types(a, b); diff --git a/src/IROperator.h b/src/IROperator.h index 048998448c75..d61362b5d13d 100644 --- a/src/IROperator.h +++ b/src/IROperator.h @@ -342,6 +342,13 @@ Expr requirement_failed_error(Expr condition, const std::vector &args); Expr memoize_tag_helper(Expr result, const std::vector &cache_key_values); +/** Compute a + widen(b). */ +Expr widen_right_add(Expr a, Expr b); +/** Compute a * widen(b). */ +Expr widen_right_mul(Expr a, Expr b); +/** Compute a - widen(b). */ +Expr widen_right_sub(Expr a, Expr b); + /** Compute widen(a) + widen(b). */ Expr widening_add(Expr a, Expr b); /** Compute widen(a) * widen(b). a and b may have different signedness. */ diff --git a/src/runtime/hvx_128.ll b/src/runtime/hvx_128.ll index 848f1ab46960..d97982deb620 100644 --- a/src/runtime/hvx_128.ll +++ b/src/runtime/hvx_128.ll @@ -145,18 +145,6 @@ define private <64 x i32> @vaslw.acc.dv.128B(<64 x i32> %a, <64 x i32> %l, i32 % ret <64 x i32> %s } -define weak_odr <64 x i32> @halide.hexagon.mul.vuw.vuh(<64 x i32> %a, <64 x i16> %b) nounwind uwtable readnone alwaysinline { - %a_lo = call <32 x i32> @llvm.hexagon.V6.lo.128B(<64 x i32> %a) - %a_hi = call <32 x i32> @llvm.hexagon.V6.hi.128B(<64 x i32> %a) - %a_e = call <32 x i32> @llvm.hexagon.V6.vshufeh.128B(<32 x i32> %a_hi, <32 x i32> %a_lo) - %a_o = call <32 x i32> @llvm.hexagon.V6.vshufoh.128B(<32 x i32> %a_hi, <32 x i32> %a_lo) - %b_32 = bitcast <64 x i16> %b to <32 x i32> - %ab_e = call <64 x i32> @llvm.hexagon.V6.vmpyuhv.128B(<32 x i32> %a_e, <32 x i32> %b_32) - %ab_o = call <64 x i32> @llvm.hexagon.V6.vmpyuhv.128B(<32 x i32> %a_o, <32 x i32> %b_32) - %ab = call <64 x i32> @vaslw.acc.dv.128B(<64 x i32> %ab_e, <64 x i32> %ab_o, i32 16) - ret <64 x i32> %ab -} - define weak_odr <64 x i32> @halide.hexagon.mul.vuw.vuw(<64 x i32> %a, <64 x i32> %b) nounwind uwtable readnone alwaysinline { %a_lo = call <32 x i32> @llvm.hexagon.V6.lo.128B(<64 x i32> %a) %a_hi = call <32 x i32> @llvm.hexagon.V6.hi.128B(<64 x i32> %a) diff --git a/test/correctness/intrinsics.cpp b/test/correctness/intrinsics.cpp index 068f360d214f..e1a892812df2 100644 --- a/test/correctness/intrinsics.cpp +++ b/test/correctness/intrinsics.cpp @@ -243,7 +243,7 @@ int main(int argc, char **argv) { check(narrow((u32(u16x) + 15) >> 4), rounding_halving_add(u16x, u16(14)) >> u16(3)); // But not if the constant can't fit in the narrower type - check(narrow((u16(u8x) + 500) >> 4), narrow((u16(u8x) + 500) >> 4)); + check(narrow((u16(u8x) + 500) >> 4), narrow((widen_right_add(cast(500), u8x)) >> 4)); check((u64(u32x) + 8) / 16, u64(rounding_shift_right(u32x, 4))); check(u16(min((u64(u32x) + 8) / 16, 65535)), u16_sat(rounding_shift_right(u32x, 4))); diff --git a/test/correctness/simd_op_check_hvx.cpp b/test/correctness/simd_op_check_hvx.cpp index fc35840bcaad..7685da369d18 100644 --- a/test/correctness/simd_op_check_hvx.cpp +++ b/test/correctness/simd_op_check_hvx.cpp @@ -458,7 +458,7 @@ class SimdOpCheckHVX : public SimdOpCheckTest { check("vmpyi(v*.h,v*.h)", hvx_width / 2, i16_1 * i16_2); check("vmpyio(v*.w,v*.h)", hvx_width / 2, i32_1 * i32(i16_1)); check("vmpyie(v*.w,v*.uh)", hvx_width / 2, i32_1 * i32(u16_1)); - check("vmpy(v*.uh,v*.uh)", hvx_width / 2, u32_1 * u32(u16_1)); + check("vmpyie(v*.w,v*.uh)", hvx_width / 2, u32_1 * u32(u16_1)); check("vmpyieo(v*.h,v*.h)", hvx_width / 4, i32_1 * i32_2); // The inconsistency in the expected instructions here is // correct. For bytes, the unsigned value is first, for half From 70c8cb89e459507575ed9f717c78396bc0fe6423 Mon Sep 17 00:00:00 2001 From: Alexander Root Date: Mon, 22 Aug 2022 15:56:08 -0400 Subject: [PATCH 2/9] clang-format --- src/CodeGen_ARM.cpp | 3 ++- src/FindIntrinsics.cpp | 5 ----- src/IROperator.cpp | 22 ++++++++++++++-------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/CodeGen_ARM.cpp b/src/CodeGen_ARM.cpp index 58ede08568ac..cf7be2ca6441 100644 --- a/src/CodeGen_ARM.cpp +++ b/src/CodeGen_ARM.cpp @@ -1206,7 +1206,8 @@ void CodeGen_ARM::visit(const Call *op) { // Look for (S | U)ADALP patterns. // This would be much easier to do if we matched on reduction factors of // VectorReduce nodes instead of the output lanes. - if (// TODO: is there a way to not just undo what FindIntrinsics does here? + if ( + // TODO: is there a way to not just undo what FindIntrinsics does here? rewrite( widen_right_add(x, h_add(y, lanes)), x + h_add(cast(op->type.with_lanes(lanes * 2), y), lanes), diff --git a/src/FindIntrinsics.cpp b/src/FindIntrinsics.cpp index f1b36ea6a710..9dead75cac79 100644 --- a/src/FindIntrinsics.cpp +++ b/src/FindIntrinsics.cpp @@ -115,7 +115,6 @@ Expr to_rounding_shift(const Call *c) { Expr a = c->args[0]; Expr b = c->args[1]; - // Helper to make the appropriate shift. auto rounding_shift = [&](const Expr &a, const Expr &b) { if (c->is_intrinsic(Call::shift_right)) { @@ -314,7 +313,6 @@ class FindIntrinsics : public IRMutator { return Add::make(a, negative_b); } - // Run after the lossless_negate check, because we want that to turn into an widen_right_add if relevant. if (op->type.is_int_or_uint() && op->type.bits() > 8) { // Look for widen_right_sub intrinsics. @@ -341,7 +339,6 @@ class FindIntrinsics : public IRMutator { } } - if (a.same_as(op->a) && b.same_as(op->b)) { return op; } else { @@ -738,7 +735,6 @@ class FindIntrinsics : public IRMutator { Type opposite_type = op->type.is_int() ? op->type.with_code(halide_type_uint) : op->type.with_code(halide_type_int); const auto is_x_wider_opposite_int = (op->type.is_int() && is_uint(x, 2 * bits)) || (op->type.is_uint() && is_int(x, 2 * bits)); - if ( // Simplify extending patterns. // (x + widen(y)) + widen(z) = x + widening_add(y, z). @@ -1071,7 +1067,6 @@ Expr lower_widen_right_mul(const Expr &a, const Expr &b) { return a * widen(b); } - Expr lower_widen_right_sub(const Expr &a, const Expr &b) { return a - widen(b); } diff --git a/src/IROperator.cpp b/src/IROperator.cpp index 9c555cd382d2..f99ab78eb3cd 100644 --- a/src/IROperator.cpp +++ b/src/IROperator.cpp @@ -1105,10 +1105,12 @@ Expr memoize_tag_helper(Expr result, const std::vector &cache_key_values) } Expr widen_right_add(Expr a, Expr b) { - user_assert(a.defined() && b.defined()) << "widen_right_add of undefined Expr\n" << a << ", " << b << "\n"; + user_assert(a.defined() && b.defined()) << "widen_right_add of undefined Expr\n" + << a << ", " << b << "\n"; user_assert(a.type().is_int_or_uint() && b.type().is_int_or_uint()) << "widen_right_add only defined for integer types, received:\n " << a << "\n " << b << "\n"; - user_assert(b.type().bits() <= 32) << "widen_right_add of large Expr\n" << a << ", " << b << "\n"; + user_assert(b.type().bits() <= 32) << "widen_right_add of large Expr\n" + << a << ", " << b << "\n"; match_lanes(a, b); Type wide_type = b.type().widen(); user_assert(wide_type == a.type()) << "widen_right_add type mismatch\n " << a << "\n " << b << "\n"; @@ -1116,10 +1118,12 @@ Expr widen_right_add(Expr a, Expr b) { } Expr widen_right_mul(Expr a, Expr b) { - user_assert(a.defined() && b.defined()) << "widen_right_mul of undefined Expr\n" << a << ", " << b << "\n"; + user_assert(a.defined() && b.defined()) << "widen_right_mul of undefined Expr\n" + << a << ", " << b << "\n"; user_assert(a.type().is_int_or_uint() && b.type().is_int_or_uint()) << "widen_right_mul only defined for integer types, received:\n " << a << "\n " << b << "\n"; - user_assert(b.type().bits() <= 32) << "widen_right_mul of large Expr\n" << a << ", " << b << "\n"; + user_assert(b.type().bits() <= 32) << "widen_right_mul of large Expr\n" + << a << ", " << b << "\n"; match_lanes(a, b); Type wide_type = b.type().widen(); user_assert(wide_type == a.type()) << "widen_right_mul type mismatch\n " << a << "\n " << b << "\n"; @@ -1127,17 +1131,19 @@ Expr widen_right_mul(Expr a, Expr b) { } Expr widen_right_sub(Expr a, Expr b) { - user_assert(a.defined() && b.defined()) << "widen_right_sub of undefined Expr\n" << a << ", " << b << "\n"; + user_assert(a.defined() && b.defined()) << "widen_right_sub of undefined Expr\n" + << a << ", " << b << "\n"; user_assert(a.type().is_int_or_uint() && b.type().is_int_or_uint()) << "widen_right_sub only defined for integer types, received:\n " << a << "\n " << b << "\n"; - user_assert(b.type().bits() <= 32) << "widen_right_sub of large Expr\n" << a << ", " << b << "\n"; + user_assert(b.type().bits() <= 32) << "widen_right_sub of large Expr\n" + << a << ", " << b << "\n"; match_lanes(a, b); Type wide_type = b.type().widen(); - user_assert(wide_type == a.type()) << "widen_right_sub type mismatch\n" << a << ", " << b << "\n"; + user_assert(wide_type == a.type()) << "widen_right_sub type mismatch\n" + << a << ", " << b << "\n"; return Call::make(wide_type, Call::widen_right_sub, {std::move(a), std::move(b)}, Call::PureIntrinsic); } - Expr widening_add(Expr a, Expr b) { user_assert(a.defined() && b.defined()) << "widening_add of undefined Expr\n"; match_types(a, b); From 81c650b5c783b626c868527b04982713fdcbaed4 Mon Sep 17 00:00:00 2001 From: Alexander Root Date: Tue, 23 Aug 2022 13:19:07 -0400 Subject: [PATCH 3/9] update HVX patterns with one-sided widening intrinsics --- src/HexagonOptimize.cpp | 32 ++++++++++++++++++++++---------- src/IROperator.cpp | 2 +- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/HexagonOptimize.cpp b/src/HexagonOptimize.cpp index 2e6b1ee23adc..bc5a6a385cdb 100644 --- a/src/HexagonOptimize.cpp +++ b/src/HexagonOptimize.cpp @@ -183,6 +183,8 @@ struct Pattern { // re-interleave the result. ReinterleaveOp0 = InterleaveResult | DeinterleaveOp0, + // TODO: All of these narrowing ops are now unused. Should we remove the flags? + NarrowOp0 = 1 << 10, // Replace operand 0 with its half-width equivalent. NarrowOp1 = 1 << 11, // Same as above, but for operand 1. NarrowOp2 = 1 << 12, @@ -720,8 +722,8 @@ class OptimizePatterns : public IRMutator { {"halide.hexagon.add_shl.vh.vh.uh", wild_u16x + (wild_u16x << wild_u16), Pattern::v65orLater}, // Non-widening multiply-accumulates with a scalar. - {"halide.hexagon.add_mul.vh.vh.b", wild_i16x + wild_i16x * wild_i16, Pattern::NarrowOp2}, - {"halide.hexagon.add_mul.vw.vw.h", wild_i32x + wild_i32x * wild_i32, Pattern::NarrowOp2}, + {"halide.hexagon.add_mul.vh.vh.b", wild_i16x + widen_right_mul(wild_i16x, wild_i8)}, + {"halide.hexagon.add_mul.vw.vw.h", wild_i32x + widen_right_mul(wild_i32x, wild_i16)}, // TODO: There's also a add_mul.vw.vw.b // This pattern is very general, so it must come last. @@ -861,14 +863,6 @@ class OptimizePatterns : public IRMutator { return mpyadds; } } - if (op->is_intrinsic(Call::widen_right_add)) { - Expr mpyadds = find_mpyadds(Add::make(op->args[0], cast(op->type, op->args[1]))); - if (mpyadds.defined()) { - return mpyadds; - } - } - - // TODO: do widen_right_adds // These intrinsics should get the default lowering, and we need to recursively mutate the // result. We don't want to let these fall through to CodeGen_Hexagon and CodeGen_LLVM, @@ -897,6 +891,15 @@ class OptimizePatterns : public IRMutator { {"halide.hexagon.mul.vw.vh", widen_right_mul(wild_i32x, wild_i16x), Pattern::ReinterleaveOp0}, {"halide.hexagon.mul.vw.vuh", widen_right_mul(wild_u32x, wild_u16x), Pattern::ReinterleaveOp0}, + // Some bad patterns are currently generated for `vrmpy` instructions. + {"halide.hexagon.acc_add_4mpy.vw.vub.b", widen_right_add(wild_i32x, cast(Int(16, 0), halide_hexagon_add_4mpy(Int(32, 0), ".vub.b", wild_u8x, wild_i32)))}, + {"halide.hexagon.acc_add_4mpy.vuw.vub.ub", widen_right_add(wild_u32x, cast(UInt(16, 0), halide_hexagon_add_4mpy(UInt(32, 0), ".vub.ub", wild_u8x, wild_u32)))}, + {"halide.hexagon.acc_add_4mpy.vuw.vub.ub", widen_right_add(wild_i32x, cast(Int(16, 0), halide_hexagon_add_4mpy(Int(32, 0), ".vub.ub", wild_u8x, wild_u32)))}, + {"halide.hexagon.acc_add_4mpy.vuw.vub.vub", widen_right_add(wild_u32x, cast(UInt(16, 0), halide_hexagon_add_4mpy(UInt(32, 0), ".vub.vub", wild_u8x, wild_u8x)))}, + {"halide.hexagon.acc_add_4mpy.vuw.vub.vub", widen_right_add(wild_i32x, cast(Int(16, 0), halide_hexagon_add_4mpy(Int(32, 0), ".vub.vub", wild_u8x, wild_u8x)))}, + {"halide.hexagon.acc_add_4mpy.vw.vub.vb", widen_right_add(wild_i32x, cast(Int(16, 0), halide_hexagon_add_4mpy(Int(32, 0), ".vub.vb", wild_u8x, wild_i8x)))}, + {"halide.hexagon.acc_add_4mpy.vw.vb.vb", widen_right_add(wild_i32x, cast(Int(16, 0), halide_hexagon_add_4mpy(Int(32, 0), ".vb.vb", wild_i8x, wild_i8x)))}, + // Saturating narrowing casts with rounding {"halide.hexagon.trunc_satub_rnd.vh", u8_sat(rounding_shift_right(wild_i16x, 8)), Pattern::DeinterleaveOp0}, {"halide.hexagon.trunc_satb_rnd.vh", i8_sat(rounding_shift_right(wild_i16x, 8)), Pattern::DeinterleaveOp0}, @@ -1001,6 +1004,15 @@ class OptimizePatterns : public IRMutator { } } + // If we failed to match on one of the patterns above, + // then search for mpyadds. + if (op->is_intrinsic(Call::widen_right_add)) { + Expr mpyadds = find_mpyadds(Add::make(op->args[0], cast(op->type, op->args[1]))); + if (mpyadds.defined()) { + return mpyadds; + } + } + if (op->is_intrinsic(Call::lerp)) { // We need to lower lerps now to optimize the arithmetic // that they generate. diff --git a/src/IROperator.cpp b/src/IROperator.cpp index f99ab78eb3cd..d008db97d8eb 100644 --- a/src/IROperator.cpp +++ b/src/IROperator.cpp @@ -480,7 +480,7 @@ Expr lossless_cast(Type t, Expr e) { } } - if ((t.is_int() || t.is_uint()) && t.bits() >= 16) { + if (t.is_int_or_uint() && t.bits() >= 16) { if (const Add *add = e.as()) { // If we can losslessly narrow the args even more // aggressively, we're good. From 95f90bc9233f297604216984dd3436a2fb175c97 Mon Sep 17 00:00:00 2001 From: Alexander Root Date: Tue, 23 Aug 2022 14:08:00 -0400 Subject: [PATCH 4/9] don't turn VectorReduce nodes into widen_right_adds --- src/CodeGen_ARM.cpp | 28 ---------------------------- src/FindIntrinsics.cpp | 5 +++-- src/HexagonOptimize.cpp | 26 +++++++------------------- 3 files changed, 10 insertions(+), 49 deletions(-) diff --git a/src/CodeGen_ARM.cpp b/src/CodeGen_ARM.cpp index cf7be2ca6441..eb4cefa670c3 100644 --- a/src/CodeGen_ARM.cpp +++ b/src/CodeGen_ARM.cpp @@ -1196,34 +1196,6 @@ void CodeGen_ARM::visit(const Call *op) { } } - auto rewrite = IRMatcher::rewriter(op, op->type); - IRMatcher::Wild<0> x; - IRMatcher::Wild<1> y; - const int bits = op->type.bits(); - const int lanes = op->type.lanes(); - auto y_2x = is_int(y, bits / 2, lanes * 2) || is_uint(y, bits / 2, lanes * 2); - - // Look for (S | U)ADALP patterns. - // This would be much easier to do if we matched on reduction factors of - // VectorReduce nodes instead of the output lanes. - if ( - // TODO: is there a way to not just undo what FindIntrinsics does here? - rewrite( - widen_right_add(x, h_add(y, lanes)), - x + h_add(cast(op->type.with_lanes(lanes * 2), y), lanes), - y_2x) || - - rewrite( - widen_right_add(x, h_add(y, lanes)), - x + h_add(cast(op->type.with_lanes(lanes * 2), h_add(y, lanes * 2)), lanes), - // y must be even to do this split. - has_even_lanes(y)) || - - false) { - value = codegen(rewrite.result); - return; - } - if (target.has_feature(Target::ARMFp16)) { auto it = float16_transcendental_remapping.find(op->name); if (it != float16_transcendental_remapping.end()) { diff --git a/src/FindIntrinsics.cpp b/src/FindIntrinsics.cpp index 9dead75cac79..d3a65db4827c 100644 --- a/src/FindIntrinsics.cpp +++ b/src/FindIntrinsics.cpp @@ -232,8 +232,9 @@ class FindIntrinsics : public IRMutator { // and the opposite as well. for (halide_type_code_t code : {op->type.code(), halide_type_uint, halide_type_int}) { Type narrow = op->type.narrow().with_code(code); - Expr narrow_a = lossless_cast(narrow, a); - Expr narrow_b = lossless_cast(narrow, b); + // Pulling casts out of VectorReduce nodes breaks too much codegen, skip for now. + Expr narrow_a = (a.node_type() == IRNodeType::VectorReduce) ? Expr() : lossless_cast(narrow, a); + Expr narrow_b = (b.node_type() == IRNodeType::VectorReduce) ? Expr() : lossless_cast(narrow, b); // This case should have been handled by the above check for widening_add. internal_assert(!(narrow_a.defined() && narrow_b.defined())) diff --git a/src/HexagonOptimize.cpp b/src/HexagonOptimize.cpp index bc5a6a385cdb..87c349a0763a 100644 --- a/src/HexagonOptimize.cpp +++ b/src/HexagonOptimize.cpp @@ -454,7 +454,6 @@ int find_mpy_ops(const Expr &op, Type a_ty, Type b_ty, int max_mpy_count, // Perform peephole optimizations on the IR, adding appropriate // interleave and deinterleave calls. class OptimizePatterns : public IRMutator { -private: using IRMutator::visit; Scope bounds; @@ -863,6 +862,13 @@ class OptimizePatterns : public IRMutator { return mpyadds; } } + // TODO: There can be better instruction selection for these. + if (op->is_intrinsic(Call::widen_right_add)) { + Expr mpyadds = find_mpyadds(Add::make(op->args[0], cast(op->type, op->args[1]))); + if (mpyadds.defined()) { + return mpyadds; + } + } // These intrinsics should get the default lowering, and we need to recursively mutate the // result. We don't want to let these fall through to CodeGen_Hexagon and CodeGen_LLVM, @@ -891,15 +897,6 @@ class OptimizePatterns : public IRMutator { {"halide.hexagon.mul.vw.vh", widen_right_mul(wild_i32x, wild_i16x), Pattern::ReinterleaveOp0}, {"halide.hexagon.mul.vw.vuh", widen_right_mul(wild_u32x, wild_u16x), Pattern::ReinterleaveOp0}, - // Some bad patterns are currently generated for `vrmpy` instructions. - {"halide.hexagon.acc_add_4mpy.vw.vub.b", widen_right_add(wild_i32x, cast(Int(16, 0), halide_hexagon_add_4mpy(Int(32, 0), ".vub.b", wild_u8x, wild_i32)))}, - {"halide.hexagon.acc_add_4mpy.vuw.vub.ub", widen_right_add(wild_u32x, cast(UInt(16, 0), halide_hexagon_add_4mpy(UInt(32, 0), ".vub.ub", wild_u8x, wild_u32)))}, - {"halide.hexagon.acc_add_4mpy.vuw.vub.ub", widen_right_add(wild_i32x, cast(Int(16, 0), halide_hexagon_add_4mpy(Int(32, 0), ".vub.ub", wild_u8x, wild_u32)))}, - {"halide.hexagon.acc_add_4mpy.vuw.vub.vub", widen_right_add(wild_u32x, cast(UInt(16, 0), halide_hexagon_add_4mpy(UInt(32, 0), ".vub.vub", wild_u8x, wild_u8x)))}, - {"halide.hexagon.acc_add_4mpy.vuw.vub.vub", widen_right_add(wild_i32x, cast(Int(16, 0), halide_hexagon_add_4mpy(Int(32, 0), ".vub.vub", wild_u8x, wild_u8x)))}, - {"halide.hexagon.acc_add_4mpy.vw.vub.vb", widen_right_add(wild_i32x, cast(Int(16, 0), halide_hexagon_add_4mpy(Int(32, 0), ".vub.vb", wild_u8x, wild_i8x)))}, - {"halide.hexagon.acc_add_4mpy.vw.vb.vb", widen_right_add(wild_i32x, cast(Int(16, 0), halide_hexagon_add_4mpy(Int(32, 0), ".vb.vb", wild_i8x, wild_i8x)))}, - // Saturating narrowing casts with rounding {"halide.hexagon.trunc_satub_rnd.vh", u8_sat(rounding_shift_right(wild_i16x, 8)), Pattern::DeinterleaveOp0}, {"halide.hexagon.trunc_satb_rnd.vh", i8_sat(rounding_shift_right(wild_i16x, 8)), Pattern::DeinterleaveOp0}, @@ -1004,15 +1001,6 @@ class OptimizePatterns : public IRMutator { } } - // If we failed to match on one of the patterns above, - // then search for mpyadds. - if (op->is_intrinsic(Call::widen_right_add)) { - Expr mpyadds = find_mpyadds(Add::make(op->args[0], cast(op->type, op->args[1]))); - if (mpyadds.defined()) { - return mpyadds; - } - } - if (op->is_intrinsic(Call::lerp)) { // We need to lower lerps now to optimize the arithmetic // that they generate. From c344e82aea044c90733b9ff673691069bfb5eafd Mon Sep 17 00:00:00 2001 From: Alexander Root Date: Tue, 23 Aug 2022 14:16:34 -0400 Subject: [PATCH 5/9] remove unused HVX pattern flags --- src/HexagonOptimize.cpp | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/src/HexagonOptimize.cpp b/src/HexagonOptimize.cpp index 87c349a0763a..60872dfbf4b4 100644 --- a/src/HexagonOptimize.cpp +++ b/src/HexagonOptimize.cpp @@ -183,20 +183,8 @@ struct Pattern { // re-interleave the result. ReinterleaveOp0 = InterleaveResult | DeinterleaveOp0, - // TODO: All of these narrowing ops are now unused. Should we remove the flags? - - NarrowOp0 = 1 << 10, // Replace operand 0 with its half-width equivalent. - NarrowOp1 = 1 << 11, // Same as above, but for operand 1. - NarrowOp2 = 1 << 12, - NarrowOps = NarrowOp0 | NarrowOp1 | NarrowOp2, - - NarrowUnsignedOp0 = 1 << 15, // Similar to the above, but narrow to an unsigned half width type. - NarrowUnsignedOp1 = 1 << 16, - NarrowUnsignedOp2 = 1 << 17, - NarrowUnsignedOps = NarrowUnsignedOp0 | NarrowUnsignedOp1 | NarrowUnsignedOp2, - - v65orLater = 1 << 21, // Pattern should be matched only for v65 target or later - v66orLater = 1 << 22, // Pattern should be matched only for v66 target or later + v65orLater = 1 << 10, // Pattern should be matched only for v65 target or later + v66orLater = 1 << 11, // Pattern should be matched only for v66 target or later }; string intrin; // Name of the intrinsic @@ -247,12 +235,6 @@ bool process_match_flags(vector &matches, int flags) { // corresponds to the bit (with operand 0 corresponding to the least // significant bit), so we can check for them all in a loop. for (size_t i = 0; i < matches.size(); i++) { - Type t = matches[i].type(); - if (flags & (Pattern::NarrowOp0 << i)) { - matches[i] = lossless_cast(t.narrow(), matches[i]); - } else if (flags & (Pattern::NarrowUnsignedOp0 << i)) { - matches[i] = lossless_cast(t.narrow().with_code(Type::UInt), matches[i]); - } if (!matches[i].defined()) { return false; } From 3bcbfe188de8947edc1f2dd61f45aaf4608b0225 Mon Sep 17 00:00:00 2001 From: Alexander Root Date: Tue, 23 Aug 2022 15:37:54 -0400 Subject: [PATCH 6/9] clang tidy --- src/HexagonOptimize.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/HexagonOptimize.cpp b/src/HexagonOptimize.cpp index 60872dfbf4b4..2fe7fc5f7000 100644 --- a/src/HexagonOptimize.cpp +++ b/src/HexagonOptimize.cpp @@ -234,8 +234,8 @@ bool process_match_flags(vector &matches, int flags) { // The Pattern::Narrow*Op* flags are ordered such that the operand // corresponds to the bit (with operand 0 corresponding to the least // significant bit), so we can check for them all in a loop. - for (size_t i = 0; i < matches.size(); i++) { - if (!matches[i].defined()) { + for (const auto &match : matches) { + if (match.defined()) { return false; } } From 938bb04c39677fbe6aba3c2e5653bab316754cdd Mon Sep 17 00:00:00 2001 From: Alexander Root Date: Wed, 24 Aug 2022 13:21:37 -0400 Subject: [PATCH 7/9] stupid bug fix --- src/HexagonOptimize.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/HexagonOptimize.cpp b/src/HexagonOptimize.cpp index 2fe7fc5f7000..73c93a292cb0 100644 --- a/src/HexagonOptimize.cpp +++ b/src/HexagonOptimize.cpp @@ -235,7 +235,7 @@ bool process_match_flags(vector &matches, int flags) { // corresponds to the bit (with operand 0 corresponding to the least // significant bit), so we can check for them all in a loop. for (const auto &match : matches) { - if (match.defined()) { + if (!match.defined()) { return false; } } From 8e3a6cf447ccb2a97bd28fdec028a45d06c68dfc Mon Sep 17 00:00:00 2001 From: Alexander Root Date: Fri, 26 Aug 2022 14:47:42 -0400 Subject: [PATCH 8/9] lower widen_right_sub/widen_right_add in HexagonOptimize --- src/HexagonOptimize.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/HexagonOptimize.cpp b/src/HexagonOptimize.cpp index 73c93a292cb0..c4ecda2fe70d 100644 --- a/src/HexagonOptimize.cpp +++ b/src/HexagonOptimize.cpp @@ -846,10 +846,11 @@ class OptimizePatterns : public IRMutator { } // TODO: There can be better instruction selection for these. if (op->is_intrinsic(Call::widen_right_add)) { - Expr mpyadds = find_mpyadds(Add::make(op->args[0], cast(op->type, op->args[1]))); - if (mpyadds.defined()) { - return mpyadds; - } + Expr lowered = Add::make(op->args[0], cast(op->type, op->args[1])); + return mutate(lowered); + } else if (op->is_intrinsic(Call::widen_right_sub)) { + Expr lowered = Sub::make(op->args[0], cast(op->type, op->args[1])); + return mutate(lowered); } // These intrinsics should get the default lowering, and we need to recursively mutate the From 2d9ebe07af7b4207513bdba2b2ee2610accefe9f Mon Sep 17 00:00:00 2001 From: Alexander Root Date: Wed, 31 Aug 2022 19:27:44 -0400 Subject: [PATCH 9/9] recursively mutate widen_right variants --- src/FindIntrinsics.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/FindIntrinsics.cpp b/src/FindIntrinsics.cpp index d3a65db4827c..9700d06e319b 100644 --- a/src/FindIntrinsics.cpp +++ b/src/FindIntrinsics.cpp @@ -265,7 +265,7 @@ class FindIntrinsics : public IRMutator { result = widen_right_add(a, narrow_b); } internal_assert(result.type() == op->type); - return result; + return mutate(result); } } } @@ -335,7 +335,7 @@ class FindIntrinsics : public IRMutator { result = widen_right_sub(a, narrow_b); } internal_assert(result.type() == op->type); - return result; + return mutate(result); } } } @@ -430,7 +430,7 @@ class FindIntrinsics : public IRMutator { result = widen_right_mul(a, narrow_b); } internal_assert(result.type() == op->type); - return result; + return mutate(result); } } }