[mlir][sparse] Simplifying assertions in fromCOO

[wrengr]

Summary

Hoisting invariant assertions to the top

[wrengr]

Unfortunately, the const in the method signature is required to be certain that the assertion in the while-loop is indeed entailed. We can safely remove it, but then the compiler wouldn't warn someone who tries to make a change that would break the entailment (i.e., by actually mutating hi).

[aartbik]

In D117408#3246376, @wrengr wrote:

Unfortunately, the const in the method signature is required to be certain that the assertion in the while-loop is indeed entailed. We can safely remove it, but then the compiler wouldn't warn someone who tries to make a change that would break the entailment (i.e., by actually mutating hi).

That makes sense, but I am not sure if our style guidelines allow the const on scalars. I would just remove it to be safe, although I agree with your reasoning (in fact, in former projects, we put const on pretty much everything that would allow it ;-)

[wrengr]

removing const

[wrengr]

In D117408#3249467, @aartbik wrote:

(in fact, in former projects, we put const on pretty much everything that would allow it ;-)

That would be nice. Much easier to reason about :)

[wrengr]

rebase

[aartbik]

Still LGTM

mlir/lib/ExecutionEngine/SparseTensorUtils.cpp

 private:
   /// Initializes sparse tensor storage scheme from a memory-resident sparse
   /// tensor in coordinate scheme. This method prepares the pointers and
   /// indices arrays under the given per-dimension dense/sparse annotations.
   /// Precondition: the `elements` must be lexicographically sorted.
   void fromCOO(const std::vector<Element<V>> &elements, uint64_t lo,
                uint64_t hi, uint64_t d) {
     // Once dimensions are exhausted, insert the numerical values.
-    assert(d <= getRank());
+    assert(d <= getRank() && hi <= elements.size());
     if (d == getRank()) {
-      assert(lo < hi && hi <= elements.size());
+      assert(lo < hi);
       values.push_back(elements[lo].value);
       return;
     }
     // Visit all elements in this interval.
     uint64_t full = 0;
-    while (lo < hi) {
-      assert(lo < elements.size() && hi <= elements.size());
+    while (lo < hi) { // If `hi` is unchanged, then `lo < elements.size()`.
       // Find segment in interval with same index elements in this dimension.
       uint64_t i = elements[lo].indices[d];
       uint64_t seg = lo + 1;
       while (seg < hi && elements[seg].indices[d] == i)
         seg++;
       // Handle segment in interval for sparse or dense dimension.
       if (isCompressedDim(d)) {
         indices[d].push_back(i);