Page MenuHomePhabricator

[mlir] Add CtPop to MathOps with lowering to LLVM
ClosedPublic

Authored by rsuderman on Dec 2 2021, 2:13 PM.

Details

Summary

math.ctpop maths to the llvm.ctpop intrinsic.

Diff Detail

Event Timeline

rsuderman created this revision.Dec 2 2021, 2:13 PM
rsuderman requested review of this revision.Dec 2 2021, 2:13 PM
ftynse added inline comments.Dec 2 2021, 2:57 PM
mlir/include/mlir/Dialect/Math/IR/MathOps.td
24

Nit: integer point -> integer.

301

Nit: CtPopOp

313

Nit: ctpop

314–315

How does it work for vectors?

316

"standard attributes" are no longer a thing.

rsuderman updated this revision to Diff 391497.Dec 2 2021, 4:48 PM

Updated for ftynse comments

rsuderman marked 5 inline comments as done.Dec 2 2021, 4:48 PM
ftynse accepted this revision.Dec 3 2021, 1:05 AM
This revision is now accepted and ready to land.Dec 3 2021, 1:05 AM
jpienaar added inline comments.
mlir/include/mlir/Dialect/Math/IR/MathOps.td
307

If using declarative ASM we generate the syntax already, so this would result in duplication I believe (like https://mlir.llvm.org/docs/Dialects/MathOps/#mathatan2-mlirmathatan2op)

314

That should already be captured in the argument constraints and displayed in table format so seems redundant here (im not actually seeing where it is verified though)

rsuderman updated this revision to Diff 392144.Dec 6 2021, 11:51 AM

Updated for jpienaar comments.

rsuderman marked 2 inline comments as done.Dec 6 2021, 11:51 AM
This revision was landed with ongoing or failed builds.Dec 6 2021, 12:02 PM
This revision was automatically updated to reflect the committed changes.