[PATCH] D59918: [Attributor] Pass infrastructure and fixpoint framework

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc requested changes to this revision.
chandlerc added a comment.
This revision now requires changes to proceed.

It looks like this is a completely new approach to attribute inference compared to the post-order function atters pass?

It'd be really good to actually spell it out nicely. Is this just version #2 of that? Can we use the same name, and put the code for this behind a flag or something to allow experimenting?

Generally, and I think this goes to the comments already made, I think this needs a design overview. This could be in the commit log, but increasingly I think we'd benefit from having a more high level practice of including design documents with commits like this. So either in the file comment of the header file for this framework, or if long/complex enough, in a markdown document referenced form the file comment, I'd like to see an overview of the design and approach you're pursuing to do more complete attribute deduction.

I also think we should be clear what is happening, so it would be good to come up with terms and define them precisely. I think "deduction" should be used more for the *computing* of attributes by reasoning about the properties of the IR. And "inference" should be adding attributes due to some extrinsic knowledge such as target library info, command line flags, etc. But totally open to different / better terminology.



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:255-261
+/// ----------------------------------------------------------------------------
+///                       Abstract Attribute Classes
+/// ----------------------------------------------------------------------------
+
+/// ----------------------------------------------------------------------------
+///                       Pass (Manager) Boilerplate
+/// ----------------------------------------------------------------------------
----------------
Note that these use doxygen syntax but aren't actually doxygen comments or terribly useful for that IMO.

I'd much rather more detailed doxygen comments on the specific types than trying to partition up the code into sections. Just my preference.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59918/new/

https://reviews.llvm.org/D59918