This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Vector] Support 0-D vectors in `CmpIOp`
ClosedPublic

Authored by michalt on Dec 6 2021, 10:46 PM.

Details

Summary

Following the example of VectorOfAnyRankOf, I've done a few changes in the
.td files to help with adding the support for the 0-D case gradually.

Diff Detail

Event Timeline

michalt created this revision.Dec 6 2021, 10:46 PM
michalt requested review of this revision.Dec 6 2021, 10:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 6 2021, 10:46 PM
ftynse accepted this revision.Dec 7 2021, 1:02 AM

LGTM. Note that MLIR does not have any requirements for transition to be anyhow gradual. It does simplify the review, but that's all.

mlir/include/mlir/IR/OpBase.td
838

Nit: please consider adding a TODO: marker with "remove this when <X> is done" or a similar comment. Otherwise, this "temporary constraint" has big chances of becoming permanent :)

841

(I know this was not introduced in the present patch): one of the Of in VectorOfAnyRankOf sounds redundant.

This revision is now accepted and ready to land.Dec 7 2021, 1:02 AM
michalt updated this revision to Diff 392654.Dec 7 2021, 11:06 PM
michalt marked an inline comment as done.

Rebase and address comments

Thanks for the review! Could you land this? (I don't have commit access)

mlir/include/mlir/IR/OpBase.td
841

This sound ok to me, as in "vector of any rank, of integers". In any case, since this is temporary, I'd prefer to just leave it as is. CCing @nicolasvasilache in case he has an opinion :)

mlir/include/mlir/IR/OpBase.td
841

The goal is to retire this and get back to simply VectorOf, the bigger the wart the simpler the cleanup?

test is broken due to:

// Warning: these must be called in their textual order of definition in the
// file to not mess up FileCheck.

fixing and landing

This revision was landed with ongoing or failed builds.Dec 12 2021, 5:29 AM
This revision was automatically updated to reflect the committed changes.

Fixing test.

Oops, sorry for that and thanks for fixing! :)