This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] Rename SparseUtils.cpp file to SparseTensorUtils.cpp
ClosedPublic

Authored by HarrietAkot on Nov 2 2021, 12:25 PM.

Details

Summary

Bug 52304 - Rename the sparse runtime support library cpp file

Diff Detail

Event Timeline

HarrietAkot created this revision.Nov 2 2021, 12:25 PM
HarrietAkot requested review of this revision.Nov 2 2021, 12:25 PM
aartbik edited the summary of this revision. (Show Details)Nov 2 2021, 12:39 PM

Much better! I updated the summary, because it is a bit strange to include a link to a file you are renaming (making it stale after this revision).
Also I think you also need to update:

clang/docs/tools/clang-formatted-files.txt

where there is one more occurrence of the "old" SparseUtils.cpp

mlir/lib/ExecutionEngine/SparseTensorUtils.cpp
1

While you are here, how about changing the contents to

//===- SparseTensorUtils.cpp - Sparse tensor utils for MLIR execution

to make it more consistent with the name

"[mlir][sparse]Editing Patch-Rename SparseUtils.cpp library"

  • "Edit the contents of SparseTensorUtils.cpp"

Did you see my comment on clang/docs/tools/clang-formatted-files.txt?
You will need to update that file too (SparseUtils.cpp -> SparseTensorUtils.cpp)

after that this revision is good to go!
(do you have submit rights, or do you want me to do that, once we are there?)

HarrietAkot marked an inline comment as done.
  • "Updating clang-formatted-files"
Herald added a project: Restricted Project. · View Herald TranscriptNov 2 2021, 1:20 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript

One last question, I just noticed you started a whole new revision (instead of updating https://reviews.llvm.org/D112995).

Was that intentional?

If you use arc, then arc diff .... first time creates one, but subsequent updates should use arc diff ... --update Dxxxx to make sure we stay in one revision.
It is not a big deal, we can proceed with this one and abandon the other, but just making sure you are aware of the procedure.

Did you see my comment on clang/docs/tools/clang-formatted-files.txt?
You will need to update that file too (SparseUtils.cpp -> SparseTensorUtils.cpp)

after that this revision is good to go!
(do you have submit rights, or do you want me to do that, once we are there?)

I have made the changes to the clang-formatted-files.txt. Kindly submit this revision for me. Thank you

mlir/lib/ExecutionEngine/SparseTensorUtils.cpp
1

Done :)

aartbik retitled this revision from "[mlir][sparse]Rename SparseUtils.cpp library to SparseTensorUtils.cpp" to [mlir][sparse] Rename SparseUtils.cpp file to SparseTensorUtils.cpp.Nov 2 2021, 1:24 PM
aartbik accepted this revision.EditedNov 2 2021, 1:36 PM

I removed the quotes from the title, no need for these. Also, please address the last open question (on arc diff --update).

But good to go as revision.

I have approved the revision and will submit this one for you. Congrats on your first contribution to sparse compilation!

This revision is now accepted and ready to land.Nov 2 2021, 1:36 PM