This is an archive of the discontinued LLVM Phabricator instance.

Remove `Ops` suffix from dialect library names
ClosedPublic

Authored by GMNGeoffrey on Sep 29 2020, 11:04 PM.

Diff Detail

Event Timeline

GMNGeoffrey created this revision.Sep 29 2020, 11:04 PM
GMNGeoffrey requested review of this revision.Sep 29 2020, 11:04 PM
GMNGeoffrey added inline comments.Sep 29 2020, 11:11 PM
mlir/docs/Tutorials/CreatingADialect.md
29

Side note: why is this file wrapped at 72 characters?

stellaraccident accepted this revision.Sep 29 2020, 11:16 PM
stellaraccident added a subscriber: stellaraccident.

I just got tripped up by the opacity of this today in npcomp. Thanks for making it more explicit. May want to give it a bit for others to respond.

This revision is now accepted and ready to land.Sep 29 2020, 11:16 PM

I just got tripped up by the opacity of this today in npcomp. Thanks for making it more explicit. May want to give it a bit for others to respond.

What opacity? Meaning, this seems like changing one convention for another, not sure what it makes more transparent. Is there documentation of the convention missing? (It was inconsistent but I never relied on me typing these directly so probably this refers to not needing to look up things)

mlir/docs/Tutorials/CreatingADialect.md
41

Did you miss this one?

GMNGeoffrey added inline comments.Sep 30 2020, 10:22 AM
mlir/docs/Tutorials/CreatingADialect.md
41

Unfortunately this actually references the td file which frequently is called FooOps :-/ There's some discussion in the discourse thread. We could make this example assume all one TD file rather than a FooOps.td file

Oh huh, I broke flang :-D I was just replacing in the MLIR directory

I just got tripped up by the opacity of this today in npcomp. Thanks for making it more explicit. May want to give it a bit for others to respond.

What opacity? Meaning, this seems like changing one convention for another, not sure what it makes more transparent. Is there documentation of the convention missing? (It was inconsistent but I never relied on me typing these directly so probably this refers to not needing to look up things)

Sorry for the half comment. "This" was referring to the multi-step cleanup referenced on the originating Discourse post.

Rebase and fix additional instance that came in.

This revision was landed with ongoing or failed builds.Sep 30 2020, 6:02 PM
This revision was automatically updated to reflect the committed changes.