From dc3be8a4756470a76baee7be30c05b906483bf3d Mon Sep 17 00:00:00 2001 From: Z Stern Date: Thu, 22 Feb 2024 02:03:10 +0000 Subject: [PATCH] Add issues for TODOs. --- src/CodeGen_LLVM.cpp | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/CodeGen_LLVM.cpp b/src/CodeGen_LLVM.cpp index e06559bbb099..f8b9e641e2da 100644 --- a/src/CodeGen_LLVM.cpp +++ b/src/CodeGen_LLVM.cpp @@ -4791,13 +4791,15 @@ Value *CodeGen_LLVM::call_intrin(const llvm::Type *result_type, int intrin_lanes arg_values[i] = builder->CreateBitCast(arg_values[i], formal_param_type); } } else { - // TODO(issue needed): That this can happen is probably a bug. It will crash in module - // validation for anything LLVM doesn't support. Better to regularize the Halide IR by - // inserting an intentional cast or to add extra intrinsics patterns. At the very least, - // some extra validation should be added here. - - // There can be some mismatches in types, such as when passing scalar Halide type T - // to LLVM vector type <1 x T>. + // TODO(https://github.com/halide/Halide/issues/8117): That this + // can happen is probably a bug. It will crash in module + // validation for anything LLVM doesn't support. Better to + // regularize the Halide IR by inserting an intentional cast or + // to add extra intrinsics patterns. At the very least, some + // extra validation should be added here. + + // There can be some mismatches in types, such as when passing + // scalar Halide type T to LLVM vector type <1 x T>. arg_values[i] = builder->CreateBitCast(arg_values[i], formal_param_type); } } @@ -4825,8 +4827,9 @@ Value *CodeGen_LLVM::slice_vector(Value *vec, int start, int size) { bool is_fixed = isa(vec->getType()); - // TODO(): It is likely worth looking into using llvm.vector.{extract,insert} - // for this case too. However that would need to be validated performance wise for all + // TODO(https://github.com/halide/Halide/issues/8118): It is likely worth + // looking into using llvm.vector.{extract,insert} for this case + // too. However that would need to be validated performance wise for all // architectures. if (is_fixed) { vector indices(size); @@ -5286,17 +5289,18 @@ llvm::Type *CodeGen_LLVM::get_vector_type(llvm::Type *t, int n, case VectorTypeConstraint::None: if (effective_vscale > 0) { bool wide_enough = true; - // TODO(): Architecture specific code should not go - // here. Ideally part of this can go away via LLVM fixes and - // modifying intrinsic selection to handle scalable vs. fixed - // vectors. Making this method virtual is possibly expensive. + // TODO(https://github.com/halide/Halide/issues/8119): Architecture + // specific code should not go here. Ideally part of this can go + // away via LLVM fixes and modifying intrinsic selection to handle + // scalable vs. fixed vectors. Making this method virtual is + // possibly expensive. if (target.arch == Target::ARM) { if (!target.has_feature(Target::NoNEON)) { - // force booleans into bytes. TODO(): figure out a better way to do this. + // force booleans into bytes. TODO(https://github.com/halide/Halide/issues/8119): figure out a better way to do this. int bit_size = std::max((int)t->getScalarSizeInBits(), 8); wide_enough = (bit_size * n) > 128; } else { - // TODO(): AArch64 SVE2 support is crashy with scalable vectors of min size 1. + // TODO(https://github.com/halide/Halide/issues/8119): AArch64 SVE2 support is crashy with scalable vectors of min size 1. wide_enough = (n / effective_vscale) > 1; } }