[mlir][sparse] Rename the public SparseTensorStorage::asCOO to toCOO

[wrengr]

Summary

Trying to reduce confusion by having the name of the public method match that of the private method for handling the recursion. Also adding some comments to SparseTensorStorage::fromCOO to help clarify what the recursive calls are doing in the dense case.

[aartbik]

Your bot build is being bitten by the "patch application failed" issue, so please make sure you have a green bot build before submitting. But changes LGTM.

[wrengr]

rebased

[wrengr]

I'm not sure how/where exactly to check on/restart the buildbot being used for this

[mehdi_amini]

I'm not sure how/where exactly to check on/restart the buildbot being used for this

Rebasing and re-running arc diff to update the revision should retrigger the build

[wrengr]

rebased and rerunning arc diff

mlir/lib/ExecutionEngine/SparseUtils.cpp

public:
   }
   void getIndices(std::vector<I> **out, uint64_t d) override {
     *out = &indices[d];
   }
   void getValues(std::vector<V> **out) override { *out = &values; }
 
   /// Returns this sparse tensor storage scheme as a new memory-resident
   /// sparse tensor in coordinate scheme with the given dimension order.
-  SparseTensor<V> *asCOO(uint64_t *perm) {
+  SparseTensor<V> *toCOO(uint64_t *perm) {
     // Restore original order of the dimension sizes and allocate coordinate
     // scheme with desired new ordering specified in perm.
     uint64_t size = getRank();
     std::vector<uint64_t> orgsz(size);
     for (uint64_t r = 0; r < size; r++)
       orgsz[rev[r]] = sizes[r];
     SparseTensor<V> *tensor = SparseTensor<V>::newSparseTensor(
         size, orgsz.data(), perm, values.size());
while (lo < hi) {
       unsigned idx = elements[lo].indices[d];
       unsigned seg = lo + 1;
       while (seg < hi && elements[seg].indices[d] == idx)
         seg++;
       // Handle segment in interval for sparse or dense dimension.
       if (sparsity[d] == kCompressed) {
         indices[d].push_back(idx);
       } else {
+        // For dense storage we must fill in all the zero values between
+        // the previous element (when last we ran this for-loop) and the
+        // current element.
         for (; full < idx; full++)
           fromCOO(tensor, sparsity, 0, 0, d + 1); // pass empty
         full++;
       }
       fromCOO(tensor, sparsity, lo, seg, d + 1);
       // And move on to next segment in interval.
       lo = seg;
     }
     // Finalize the sparse pointer structure at this dimension.
     if (sparsity[d] == kCompressed) {
       pointers[d].push_back(indices[d].size());
     } else {
+      // For dense storage we must fill in all the zero values after
+      // the last element.
       for (uint64_t sz = tensor->getSizes()[d]; full < sz; full++)
         fromCOO(tensor, sparsity, 0, 0, d + 1); // pass empty
     }
   }
 
   /// Stores the sparse tensor storage scheme into a memory-resident sparse
   /// tensor in coordinate scheme.
   void toCOO(SparseTensor<V> *tensor, std::vector<uint64_t> &reord,
if (ptrTp == (p) && indTp == (i) && valTp == (v)) {                          \
     SparseTensor<V> *tensor;                                                   \
     if (action == 0)                                                           \
       tensor = openTensor<V>(static_cast<char *>(ptr), asize, sizes, perm);    \
     else if (action == 1)                                                      \
       tensor = static_cast<SparseTensor<V> *>(ptr);                            \
     else if (action == 2)                                                      \
       return SparseTensor<V>::newSparseTensor(asize, sizes, perm);             \
     else                                                                       \
-      return static_cast<SparseTensorStorage<P, I, V> *>(ptr)->asCOO(perm);    \
+      return static_cast<SparseTensorStorage<P, I, V> *>(ptr)->toCOO(perm);    \
     return SparseTensorStorage<P, I, V>::newSparseTensor(tensor, sparsity,     \
                                                          perm);                \
   }
 
 #define IMPL1(RET, NAME, TYPE, LIB)                                            \
   RET NAME(void *tensor) {                                                     \
     std::vector<TYPE> *v;                                                      \
     static_cast<SparseTensorStorageBase *>(tensor)->LIB(&v);                   \