This patch updates the code generation test tool, which will be used in subsequent patches to test the FIR ops, lowering them to LLVM IR, etc. It also includes a number of files with support code which will be used by code generation as pieces are added in future patches.
I have concerns with such approach that makes the compilation flow very stateful, where things like a Triple can easily be encoded in the IR as an attribute for example.
Seems like all the header guards need an update.
There is no loading involved, the function should be renamed here (and the doc updated)
Make a note that registration is only ever useful to be able to parse the passes from a textual string (mostly in testing tools) and not to construct a pass pipeline.
Do we need two functions to do the same thing? Can you keep one?
Opaque attribute are supposed to be non-semantically impactful I believe and be able to be dropped, Triple does not seems to fit here.
I actually don't know about uniquer and kindmap, but this is making me nervous as well.
Your objections are noted. We'd like to table this for the moment so as to continue upstreaming.
This is unexpected as it follows the same pattern used in previously upstreamed headers. We'll figure out what changed in clang-tidy.
Right. The change to registration was made a day or two ago.
We'll look into it.
This function is just basically "renaming" llvm::report_fatal_error? It isn't clear to me why we wouldn't just use llvm::report_fatal_error?
Seems like your patch update messed up Phabricator comments handling, I can't see your answer here, but I think you wrote:
The point of reviewing this kind of thing from your out-of-tree prototype to upstream is to address this. We can't just table this.
(Also I don't see anything testing the Opaque attributes right now.)