This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Remove support for non-prefixed accessors
ClosedPublic

Authored by rriddle on Oct 25 2022, 6:31 PM.

Details

Summary

This finishes off a year long pursuit to LLVMify the generated
operation accessors, prefixing them with get/set. Support for
any other accessor naming is fully removed after this commit.

https://discourse.llvm.org/t/psa-raw-accessors-are-being-removed/65629

Diff Detail

Event Timeline

rriddle created this revision.Oct 25 2022, 6:31 PM
Herald added a reviewer: aartbik. · View Herald Transcript
Herald added a reviewer: ftynse. · View Herald Transcript
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
rriddle requested review of this revision.Oct 25 2022, 6:31 PM
rriddle updated this revision to Diff 470675.Oct 25 2022, 6:35 PM
tpopp added a subscriber: tpopp.Oct 26 2022, 1:19 AM

Are you still going to wait until early November, or are you going to commit it this week?

Are you still going to wait until early November, or are you going to commit it this week?

I'll wait until early November (like in the post), just sent this out to prepare for that.

Mogball accepted this revision.Oct 27 2022, 11:54 AM

The End of an Era

This revision is now accepted and ready to land.Oct 27 2022, 11:54 AM
tpopp added a comment.Oct 31 2022, 4:26 PM

Are you still going to wait until early November, or are you going to commit it this week?

I'll wait until early November (like in the post), just sent this out to prepare for that.

Thank you, and I hope my question wasn't too abrupt, as I just wanted to check while preparing for handling this in my org.

Are you still going to wait until early November, or are you going to commit it this week?

I'll wait until early November (like in the post), just sent this out to prepare for that.

Thank you, and I hope my question wasn't too abrupt, as I just wanted to check while preparing for handling this in my org.

No worries at all!

Do you think we could wait until after the LLVM Dev meeting MLIR Summit to land this? We're kind of scrambling around these days and still working on preparing for this to land.

Do you think we could wait until after the LLVM Dev meeting MLIR Summit to land this? We're kind of scrambling around these days and still working on preparing for this to land.

Yeah, that's fine.

tpopp added a comment.Nov 10 2022, 6:43 PM

Do you think we could wait until after the LLVM Dev meeting MLIR Summit to land this? We're kind of scrambling around these days and still working on preparing for this to land.

Yeah, that's fine.

Thank you very much for waiting a while more on this.

jpienaar accepted this revision.Nov 26 2022, 7:15 AM

Nice amount of red :-)

mlir/docs/DefiningDialects.md
305

Is this recorded somewhere still post? This is default now.

We may also want to warn when attributes overlap with more helpful message in the case where it is due to this transform, e.g., t and T now map to same accessor where it didn't before and some renaming would be needed (a bit unfortunate that I didn't add that check earlier). [Not blocker, could just be GitHub issue too]

rriddle marked an inline comment as done.Dec 2 2022, 12:36 PM
rriddle added inline comments.
mlir/docs/DefiningDialects.md
305

I think we do emit an error if we generate an overlap, but not sure it's the nicest error.

Added back a blurb on accessor generation.

This revision was automatically updated to reflect the committed changes.
rriddle marked an inline comment as done.