This is an archive of the discontinued LLVM Phabricator instance.

[mlir][sparse] unifying enterLoopOverTensorAtLvl and enterCoIterationOverTensorsAtLvls
ClosedPublic

Authored by Peiming on Jun 8 2023, 12:33 PM.

Details

Summary

The tensor levels are now explicitly categorized into different LoopCondKind to instruct LoopEmitter generate different code for different kinds of condition (e.g., SparseCond, SparseSliceCond, SparseAffineIdxCond, etc)

The process of generating a while loop is now dissembled into three steps and they are dispatched to different LoopCondKind handler.

  1. Generate LoopCondition (e.g., pos <= posHi for SparseCond, slice.isNonEmpty for SparseAffineIdxCond)
  2. Generate LoopBody (e.g., compute the coordinates)
  3. Generate ExtraChecks (e.g., if (onSlice(crd)) for SparseSliceCond)

Diff Detail

Event Timeline

Peiming created this revision.Jun 8 2023, 12:33 PM
Herald added a project: Restricted Project. · View Herald Transcript
Peiming requested review of this revision.Jun 8 2023, 12:33 PM
Peiming edited the summary of this revision. (Show Details)Jun 8 2023, 12:53 PM
Peiming updated this revision to Diff 529793.Jun 8 2023, 5:39 PM

fix bugs.

Peiming planned changes to this revision.Jun 8 2023, 5:39 PM
Peiming added inline comments.Jun 8 2023, 5:42 PM
mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1469

How come that this did not cause any issue before?

Peiming updated this revision to Diff 530572.Jun 12 2023, 10:08 AM

excludes unrelated changes.

aartbik added inline comments.Jun 12 2023, 12:10 PM
mlir/lib/Dialect/SparseTensor/Transforms/LoopEmitter.h
144

Add comment, since we document all methods pretty well in this header

148

same here, and below

270

.... that hold different

294

"that the used" seems to be missing something

299

same, can be used?

331

loop conditions

mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1308–1309

since this returns a bool, I would just say

isParallelLoop(....)

or so (since isParallelFor is already taken)

Peiming updated this revision to Diff 530724.Jun 12 2023, 5:11 PM
Peiming marked 7 inline comments as done.

address comments.

mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1308–1309

How about the current name?

aartbik added inline comments.Jun 14 2023, 10:34 AM
mlir/lib/Dialect/SparseTensor/Transforms/LoopEmitter.h
144–147

.. loop that tries to ...

270

I still see "holds" not "hold" (subject is plural). so please use hold

mlir/lib/Dialect/SparseTensor/Transforms/Sparsification.cpp
1308–1309

works too

1309–1312

add

/// Comment

1327

I liked the original genFor and genWhile terminology here, since it was really what was generated
(the filter and coiteration live less for me)

Peiming updated this revision to Diff 531427.Jun 14 2023, 10:54 AM
Peiming marked 3 inline comments as done.

address comments.

Peiming updated this revision to Diff 531428.Jun 14 2023, 10:56 AM

update comments.

aartbik accepted this revision.Jun 14 2023, 12:25 PM
This revision is now accepted and ready to land.Jun 14 2023, 12:25 PM
This revision was landed with ongoing or failed builds.Jun 14 2023, 1:03 PM
This revision was automatically updated to reflect the committed changes.