LG with fixing the wording as Alex suggests (and clang-tidy flags)

Oh I see what you mean, we'd replace the current llvm::StringSet<llvm::BumpPtrAllocator &> identifiers; in the MLIRContext with something like llvm::StringMap<Dialect*, llvm::BumpPtrAllocator &> identifiers;?
(probably a bit more complex, but that's the rough direction?)

This patch is fine, but I'd still be interested to understand if this could be made to show up during check-mlir with one of the Cmake configuration that can run on Windows?

Some Locations may be using Identifier like in FileLineColLocationStorage.
An OperationName would get larger as well I think, which would make Operation larger by one pointer (it'd accelerate Operation::getDialect() for unregistered operations, but I don't think we need to optimize this)

In D95418#2524202, @jpienaar wrote:

Isn't it still just an Identifier? The dialect was always there, it was just previously encoded in the string and is now separate

In D95394#2521877, @stephenneuendorffer wrote:

This seems to me like a 'tip of the iceberg' kind of solution. What happens if the attribute needs to be transformed? What happens if there is not a 1:1 correspondance between input operations and output operations in a transformation?

Thanks for the cleanup!

You can search in mlir/test for test that invoke mlir-translate with -mlir-to-llvmir

Thanks! It is exciting to start seeing the FIR change coming upstream!

In D92898#2520601, @jpienaar wrote:

I know there was another prototype in this region, does that subsume this?

LGTM
(fix the description before pushing)

LGTM with a minor tweak on FileReproducerStream

Landing now, feel free to add more comments as needed!

Add a body_builder callback

Can you add a lit test for this?

LGTM

Do you need me to land this for you?

Minor update to add_entry_block + test fix

Address comment, add more helpers/properties on FuncOp, extend the test

Thanks!

Since the split of the standard dialect has begun, can you create a math dialect and move this kind of operations there?

We don't need to test completeness: i.e. no need to test every feature from ODS, just basic plumbing here (import and create / round-trip one or two ops), that's why I mentioned "smoke test".
I guess it'll be more useful with the registration aspect sorted out.

I think it'd be preferable to write basic smoke tests for the bindings when adding these.

In D94918#2505438, @mehdi_amini wrote:

We generally use code review for everything except build unbreaks, reverts and minor syntactic changes such as typos.

I have a more liberal view here, I've been performing refactoring and other kind of NFC or minor change without pre-commit reviews. In general many people (including me) are just reviewing a few things post-commit.

We generally use code review for everything except build unbreaks, reverts and minor syntactic changes such as typos.

Phab linked the commit as "added a commit" to this revision, I'm not sure why it didn't close it...

LGTM in general.

Sweet!

If the runners depends on libSupport and we link them statically into the runtime wrapper that are dynamically loaded, aren't we having two copies of libSupport? One in the statically linked runner executable and one in the dynamically loaded shared objects?

Thanks for adding the description: I still don't quite get how making these static is preventing an ODR issues?

Thanks Alex!

Sorry I had to revert in 110775809ad1 as it broke the gcc-5 build (which has been broken for hours by another change that just got reverted, so I didn't want to lose more coverage right now and need to get it back green ASAP).

Can you add a description of the motivation to the commit message?

LGTM

LGTM, Thanks!

LGTM, Thanks!

Still LGTM! Do you have commit access or do you need someone to land it for you?

LGTM

awesome :)

Fixed in 476db17dcb64ef3ec6e247f4b1c673b57f61a367 and 9e1aaa9943b814c22ae03f4abb3171dac8062801

To bisect a build failure, is is more reliable to just build the target that is failing, i.e.: ninja projects/debuginfo-tests/CMakeFiles/check-gdb-mlir-support.dir/llvm-prettyprinters/gdb/mlir-support.cpp.o