This is an archive of the discontinued LLVM Phabricator instance.

[mlir] FunctionOpInterface: arg and result attrs dispatch to interface
ClosedPublic

Authored by Mogball on Dec 6 2022, 2:52 PM.

Details

Summary

This patch removes the arg_attrs and res_attrs named attributes as a
requirement for FunctionOpInterface and replaces them with interface
methods for the getters, setters, and removers of the relevent
attributes. This allows operations to use their own storage for the
argument and result attributes.

Depends on D139471

Diff Detail

Event Timeline

Mogball created this revision.Dec 6 2022, 2:52 PM
Mogball requested review of this revision.Dec 6 2022, 2:52 PM
rriddle accepted this revision.Dec 8 2022, 10:33 AM

Seems reasonable to me!

This revision was not accepted when it landed; it landed in state Needs Review.Dec 8 2022, 11:33 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.

@Mogball, this change breaks the flang build. Can you please either revert it or take some other action that fixes the flang build?

Sending out a fix.

Can you revert this until builds are fixed? By the way the pre-commit checks were failing as well before it landed.

I have reverted this and the follow up fix in https://github.com/llvm/llvm-project/commit/f3379feabe38fd3711b13ffcf6de4aab03b7ccdc

Here is an example of the failures: https://lab.llvm.org/buildbot/#/builders/179/builds/5050

You can reproduce with just ninja check-flang on any arch I expect. The llvm-test-suite failures look like the same root cause so no need to run those again, we'll follow up if they do break again.

And now I broke the Nvidia builders because more commits went into the examples to fix them. Should have that fixed shortly.

In future I really recommend reverting to green first, even if you think you can fix it in a small change. Especially if you are at the end of a work day, even if you aren't, sometimes things take a lot longer than you'd expect.

And now I broke the Nvidia builders because more commits went into the examples to fix them. Should have that fixed shortly.

In future I really recommend reverting to green first, even if you think you can fix it in a small change. Especially if you are at the end of a work day, even if you aren't, sometimes things take a lot longer than you'd expect.

@DavidSpickett, my builds and tests look good after your latest changes.

Two more earlier changes reverted in https://github.com/llvm/llvm-project/commit/cf98e8273c5f1c84afcfdd9bcea486ec22f26768 (which is my mistake, I should have checked the stack here first).

So far MLIR bots are happy with it. Our flang bots will catch up soon.