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

Wed Apr 17 23:43:14 PDT 2019

jdoerfert added a comment.

In D59918#1469948 <https://reviews.llvm.org/D59918#1469948>, @chandlerc wrote:

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

It is, one of the reasons I put it in a separate file. " post-order function atters" is actually 5-7 separate "passes" over an SCC just living in the same LLVM-IR pass.

> It'd be really good to actually spell it out nicely.

Good point. I guess I can put that in the commit message and also work on other descriptions.

> 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?

I "kinda" could, I prefer not to. There are various justifications, let me give two now and it that is not sufficient I'll provide more:

1. "functionattrs" outgrow "function attributes" a while ago when it started to derive argument attributes. The attributor framework is designed to derive everything, from function, to argument and return attributes, over instruction "attributes/metadata" to things that do not have an LLVM-IR equivalent.
2. The design, as well as the capabilities, of the function attribute deduction pass is fairly specific. I would like to prevent "reuse" in either direction as the Attributor framework is supposed to replace the existing code with better deduction across the board.

> 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 totally agree, as I did with Florian who requested better documentation. I added a first draft of an high-level file comment today. I will also provide a more elaborate commit message and class comments asap. Some clang cuda bugs kept me busy all day :(

> I also think we should be clear what is happening, so it would be good to come up with terms and define them precisely.

Seems very reasonable to me. I won't claim I have a clear terminology throughout the commits/comments.

> 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.

Fine with me. I think I exclusively used "deduction" (so far) as it the expected usage model. I should probably add some Attributor documentation which could also specify how we talk about things. Until then, please let me know if there is anything you want me to clarify!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59918