[Mlir-commits] [mlir] f9da447 - [mlir][sparse] clean up a few TODOs and layout
Aart Bik
llvmlistbot at llvm.org
Tue Aug 1 17:06:14 PDT 2023
Author: Aart Bik
Date: 2023-08-01T17:06:05-07:00
New Revision: f9da447fa791c078dcae52e48dcd036d7e2e539c
URL: https://github.com/llvm/llvm-project/commit/f9da447fa791c078dcae52e48dcd036d7e2e539c
DIFF: https://github.com/llvm/llvm-project/commit/f9da447fa791c078dcae52e48dcd036d7e2e539c.diff
LOG: [mlir][sparse] clean up a few TODOs and layout
Removed some TODOs that are not likely to be done
or are already recorded through git tracker.
Reviewed By: Peiming
Differential Revision: https://reviews.llvm.org/D156836
Added:
Modified:
mlir/include/mlir/Dialect/SparseTensor/IR/CMakeLists.txt
mlir/include/mlir/Dialect/SparseTensor/IR/Enums.h
mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorAttrDefs.td
mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorOps.td
mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorStorageLayout.h
mlir/include/mlir/Dialect/SparseTensor/Transforms/Passes.h
Removed:
################################################################################
diff --git a/mlir/include/mlir/Dialect/SparseTensor/IR/CMakeLists.txt b/mlir/include/mlir/Dialect/SparseTensor/IR/CMakeLists.txt
index b901eb8997cd87..25a2e4869cc782 100644
--- a/mlir/include/mlir/Dialect/SparseTensor/IR/CMakeLists.txt
+++ b/mlir/include/mlir/Dialect/SparseTensor/IR/CMakeLists.txt
@@ -12,4 +12,3 @@ set(LLVM_TARGET_DEFINITIONS SparseTensorTypes.td)
mlir_tablegen(SparseTensorTypes.h.inc -gen-typedef-decls)
mlir_tablegen(SparseTensorTypes.cpp.inc -gen-typedef-defs)
add_public_tablegen_target(MLIRSparseTensorTypesIncGen)
-
diff --git a/mlir/include/mlir/Dialect/SparseTensor/IR/Enums.h b/mlir/include/mlir/Dialect/SparseTensor/IR/Enums.h
index b0348ed8eedcf1..4b601ff7b5772f 100644
--- a/mlir/include/mlir/Dialect/SparseTensor/IR/Enums.h
+++ b/mlir/include/mlir/Dialect/SparseTensor/IR/Enums.h
@@ -42,7 +42,6 @@ namespace sparse_tensor {
/// values with the built-in type "index". For now, we simply assume that
/// type is 64-bit, but targets with
diff erent "index" bitwidths should
/// link with an alternatively built runtime support library.
-// TODO: support such targets?
using index_type = uint64_t;
/// Encoding of overhead types (both position overhead and coordinate
@@ -92,11 +91,6 @@ enum class PrimaryType : uint32_t {
};
// This x-macro includes all `V` types.
-// TODO: We currently split out the non-variadic version from the variadic
-// version. Using ##__VA_ARGS__ to avoid the split gives
-// warning: token pasting of ',' and __VA_ARGS__ is a GNU extension
-// [-Wgnu-zero-variadic-macro-arguments]
-// and __VA_OPT__(, ) __VA_ARGS__ requires c++20.
#define MLIR_SPARSETENSOR_FOREVERY_V(DO) \
DO(F64, double) \
DO(F32, float) \
@@ -205,7 +199,6 @@ enum class LevelFormat : uint8_t {
/// Returns string representation of the given dimension level type.
constexpr const char *toMLIRString(DimLevelType dlt) {
switch (dlt) {
- // TODO: should probably raise an error instead of printing it...
case DimLevelType::Undef:
return "undef";
case DimLevelType::Dense:
diff --git a/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorAttrDefs.td b/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorAttrDefs.td
index ad42d6467fa120..8f116e6355cf44 100644
--- a/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorAttrDefs.td
+++ b/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorAttrDefs.td
@@ -153,9 +153,7 @@ def SparseTensorEncodingAttr : SparseTensor_Attr<"SparseTensorEncoding",
stored with compression while dense storage is used within each block
(although hybrid schemes are possible as well).
- TODO: the following example is out-of-date and will be implemented
- in a
diff erent manner than described here.
- (This will be corrected in an upcoming change that completely
+ (The following will be corrected in an upcoming change that completely
overhauls the syntax of this attribute.)
The dimToLvl mapping also provides a notion of "counting a
@@ -439,11 +437,11 @@ def AnyRankedSparseTensor : RankedSparseTensorOf<[AnyType]>;
// Sparse Tensor Sorting Algorithm Attribute.
//===----------------------------------------------------------------------===//
-// TODO: Currently, we only provide four implementations, and expose the
-// implementations via attribute algorithm. In the future, if we will need
-// to support both stable and non-stable quick sort, we may add
-// quick_sort_nonstable enum to the attribute. Alternative, we may use two
-// attributes, (stable|nonstable, algorithm), to specify a sorting
+// Currently, we only provide four implementations, and expose the
+// implementations via attribute algorithm. In the future, if we will
+// need to support both stable and non-stable quick sort, we may add
+// quick_sort_nonstable enum to the attribute. Alternative, we may use
+// two attributes, (stable|nonstable, algorithm), to specify a sorting
// implementation.
//
// --------------------------------------------------------------------------
diff --git a/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorOps.td b/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorOps.td
index 01320a2e4c5acb..5395a0e088c90d 100644
--- a/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorOps.td
+++ b/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorOps.td
@@ -762,14 +762,6 @@ def SparseTensor_OutOp : SparseTensor_Op<"out", []>,
//===----------------------------------------------------------------------===//
def SparseTensor_SortOp : SparseTensor_Op<"sort", [AttrSizedOperandSegments]>,
- // TODO: May want to extend tablegen with
- // class NonemptyVariadic<Type type> : Variadic<type> { let minSize = 1; }
- // and then use NonemptyVariadic<...>:$xs here.
- //
- // TODO: Currently tablegen doesn't support the assembly syntax when
- // `algorithm` is an optional enum attribute. We may want to use an optional
- // enum attribute when this is fixed in tablegen.
- //
Arguments<(ins Index:$n,
Variadic<StridedMemRefRankOf<[AnyInteger, Index], [1]>>:$xs,
Variadic<StridedMemRefRankOf<[AnyType], [1]>>:$ys,
diff --git a/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorStorageLayout.h b/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorStorageLayout.h
index 82c618efa984ff..b7ce44d877eb29 100644
--- a/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorStorageLayout.h
+++ b/mlir/include/mlir/Dialect/SparseTensor/IR/SparseTensorStorageLayout.h
@@ -107,7 +107,7 @@ using FieldIndex = unsigned;
/// encoding.
class StorageLayout {
public:
- // TODO: Functions/methods marked with [NUMFIELDS] might should use
+ // TODO: Functions/methods marked with [NUMFIELDS] should use
// `FieldIndex` for their return type, via the same reasoning for why
// `Dimension`/`Level` are used both for identifiers and ranks.
explicit StorageLayout(const SparseTensorType &stt)
@@ -154,12 +154,12 @@ class StorageLayout {
// Wrapper functions to invoke StorageLayout-related method.
//
-// TODO: See note [NUMFIELDS].
+// See note [NUMFIELDS].
inline unsigned getNumFieldsFromEncoding(SparseTensorEncodingAttr enc) {
return StorageLayout(enc).getNumFields();
}
-// TODO: See note [NUMFIELDS].
+// See note [NUMFIELDS].
inline unsigned getNumDataFieldsFromEncoding(SparseTensorEncodingAttr enc) {
return StorageLayout(enc).getNumDataFields();
}
diff --git a/mlir/include/mlir/Dialect/SparseTensor/Transforms/Passes.h b/mlir/include/mlir/Dialect/SparseTensor/Transforms/Passes.h
index 2949397214dfd6..8d632bf7453adb 100644
--- a/mlir/include/mlir/Dialect/SparseTensor/Transforms/Passes.h
+++ b/mlir/include/mlir/Dialect/SparseTensor/Transforms/Passes.h
@@ -41,7 +41,6 @@ enum class SparseParallelizationStrategy {
kAnyStorageOuterLoop,
kDenseAnyLoop,
kAnyStorageAnyLoop
- // TODO: support reduction parallelization too?
};
// TODO : Zero copy is disabled due to correctness bugs.Tracker #64316
More information about the Mlir-commits
mailing list