diff --git a/src/CodeGen_LLVM.cpp b/src/CodeGen_LLVM.cpp index 31dddc3551af..a30b30ab3276 100644 --- a/src/CodeGen_LLVM.cpp +++ b/src/CodeGen_LLVM.cpp @@ -234,9 +234,6 @@ void CodeGen_LLVM::initialize_llvm() { for (const std::string &s : arg_vec) { c_arg_vec.push_back(s.c_str()); } - // TODO: Remove after opaque pointers become the default in LLVM. - // This is here to document how to turn on opaque pointers, for testing, in LLVM 15 - // c_arg_vec.push_back("-opaque-pointers"); cl::ParseCommandLineOptions((int)(c_arg_vec.size()), &c_arg_vec[0], "Halide compiler\n"); } diff --git a/src/Deinterleave.cpp b/src/Deinterleave.cpp index 255b2a67a339..54199485fed5 100644 --- a/src/Deinterleave.cpp +++ b/src/Deinterleave.cpp @@ -223,7 +223,7 @@ class Deinterleaver : public IRGraphMutator { } } if (op->value.type().lanes() > 1) { - // There is probably a more efficient way to this. + // There is probably a more efficient way to do this. return mutate(flatten_nested_ramps(op)); } @@ -236,7 +236,14 @@ class Deinterleaver : public IRGraphMutator { } else { Type t = op->type.with_lanes(new_lanes); ModulusRemainder align = op->alignment; - // TODO: Figure out the alignment of every nth lane + // The alignment of a Load refers to the alignment of the first + // lane, so we can preserve the existing alignment metadata if the + // deinterleave is asking for any subset of lanes that includes the + // first. Otherwise we just drop it. We could check if the index is + // a ramp with constant stride or some other special case, but if + // that's the case, the simplifier is very good at figuring out the + // alignment, and it has access to context (e.g. the alignment of + // enclosing lets) that we do not have here. if (starting_lane != 0) { align = ModulusRemainder(); } diff --git a/src/IRMatch.h b/src/IRMatch.h index afc264582651..676bd037bd27 100644 --- a/src/IRMatch.h +++ b/src/IRMatch.h @@ -1320,11 +1320,6 @@ constexpr bool and_reduce(bool first, Args... rest) { return first && and_reduce(rest...); } -// TODO: this can be replaced with std::min() once we require C++14 or later -constexpr int const_min(int a, int b) { - return a < b ? a : b; -} - template struct OptionalIntrinType { bool check(const Type &) const { @@ -1413,7 +1408,7 @@ struct Intrin { return saturating_cast(optional_type_hint.type, std::move(arg0)); } - Expr arg1 = std::get(args).make(state, type_hint); + Expr arg1 = std::get(1, sizeof...(Args) - 1)>(args).make(state, type_hint); if (intrin == Call::absd) { return absd(std::move(arg0), std::move(arg1)); } else if (intrin == Call::widen_right_add) { @@ -1448,7 +1443,7 @@ struct Intrin { return rounding_shift_right(std::move(arg0), std::move(arg1)); } - Expr arg2 = std::get(args).make(state, type_hint); + Expr arg2 = std::get(2, sizeof...(Args) - 1)>(args).make(state, type_hint); if (intrin == Call::mul_shift_right) { return mul_shift_right(std::move(arg0), std::move(arg1), std::move(arg2)); } else if (intrin == Call::rounding_mul_shift_right) { diff --git a/src/IROperator.cpp b/src/IROperator.cpp index 41ea946e10f4..26869c4d8b15 100644 --- a/src/IROperator.cpp +++ b/src/IROperator.cpp @@ -1048,49 +1048,41 @@ Expr strided_ramp_base(const Expr &e, int stride) { namespace { -struct RemoveLikelies : public IRMutator { +// Replace a specified list of intrinsics with their first arg. +class RemoveIntrinsics : public IRMutator { using IRMutator::visit; + const std::initializer_list &ops; + Expr visit(const Call *op) override { - if (op->is_intrinsic(Call::likely) || - op->is_intrinsic(Call::likely_if_innermost)) { + if (op->is_intrinsic(ops)) { return mutate(op->args[0]); } else { return IRMutator::visit(op); } } -}; -// TODO: There could just be one IRMutator that can remove -// calls from a list. If we need more of these, it might be worth -// doing that refactor. -struct RemovePromises : public IRMutator { - using IRMutator::visit; - Expr visit(const Call *op) override { - if (op->is_intrinsic(Call::promise_clamped) || - op->is_intrinsic(Call::unsafe_promise_clamped)) { - return mutate(op->args[0]); - } else { - return IRMutator::visit(op); - } +public: + RemoveIntrinsics(const std::initializer_list &ops) + : ops(ops) { } }; } // namespace Expr remove_likelies(const Expr &e) { - return RemoveLikelies().mutate(e); + return RemoveIntrinsics({Call::likely, Call::likely_if_innermost}).mutate(e); } Stmt remove_likelies(const Stmt &s) { - return RemoveLikelies().mutate(s); + return RemoveIntrinsics({Call::likely, Call::likely_if_innermost}).mutate(s); } Expr remove_promises(const Expr &e) { - return RemovePromises().mutate(e); + return RemoveIntrinsics({Call::promise_clamped, Call::unsafe_promise_clamped}).mutate(e); } Stmt remove_promises(const Stmt &s) { - return RemovePromises().mutate(s); + return RemoveIntrinsics({Call::promise_clamped, Call::unsafe_promise_clamped}).mutate(s); } Expr unwrap_tags(const Expr &e) { diff --git a/src/Var.h b/src/Var.h index 33207d3cea0d..fbb976b476d9 100644 --- a/src/Var.h +++ b/src/Var.h @@ -25,7 +25,9 @@ class Var { Expr e; public: - /** Construct a Var with the given name */ + /** Construct a Var with the given name. Unlike Funcs, this will be treated + * as the same Var as another other Var with the same name, including + * implicit Vars. */ Var(const std::string &n); /** Construct a Var with an automatically-generated unique name. */ @@ -120,9 +122,6 @@ class Var { static Var implicit(int n); /** Return whether a variable name is of the form for an implicit argument. - * TODO: This is almost guaranteed to incorrectly fire on user - * declared variables at some point. We should likely prevent - * user Var declarations from making names of this form. */ //{ static bool is_implicit(const std::string &name);