Thanks for implementing this, LGTM!

Thanks for the explanation! Still looking good apart from the function names ;)

LGTM, thanks for working on this! (please fix the name in CommonArgs.h before landing this)

Address Cullen's comments

Thanks for the review @c-rhodes , sending updates shortly.

Thanks for the updates! A few more pointers, but nothing major.

Thanks, LGTM!

Fix verify-flags.mlir

Thanks for the updates, mostly looks good. Just a couple of extra questions about the test coverage.

LGTM, thanks! As @kiranchandramohan points out, this is in line with the overall direction that we agreed on a while back.

What's the overall design goal here? 100% consistency with Clang? Could this be documented?

Fix Toy examples builds

Thanks for submitting this! Please add tests for -g and all variants of -gline-tables-only.

LGTM!

LGTM, thanks!

Address Diego's comments

This is a very nice patch, thanks for working on this! A few final nits, but feel free to ignore. LGTM

A few more comments, but mostly nits. Btw, is this patch sufficient to generate code for AMDGPU? Or, put differently, what's the level of support atm?

LGTM, thanks!

Really nice to see some shared code being elevated out of Clang into LLVM, thanks!

Thanks for the updates!

LGTM, thanks for improving this!

In D146333#4204216, @mnadeem wrote:

In D146333#4204007, @awarzynski wrote:

We already have an error message, so print it as well.

I'm not sure I follow - can you elaborate? This patch is switching from an assertion to a diagnostic. The error message inside the assert will be printed regardless of whether a diagnostic is used or not.

Before we were failing the assert with just a fixed message: "Failed to create Target".
With this patch we will also print the error description stored in the string error and then abort.

We already have an error message, so print it as well.

Could you add some tests?

LGTM, thanks for working on this! Would be great for somebody with a bit more experience with offloading to OK this as well :) @tschuett or perhaps @jhuber6 ?

Thanks for checking - something for a TODO list!

Thanks! Would replacing llvm-readelf -h with llvm-objdump -f also work? I'd rather avoid new dependencies if possible.

In D146075#4196998, @tschuett wrote:

What is the policy on trivial braces in the frontend?

Please add tests ;)

In D143572#4192645, @jsjodin wrote:

@sunshaoce, @awarzynski are there remaining issues with this patch?

Thanks for improving these improvements @vzakhari !

LGTM, thanks for contributing!

Fix the issue of .o file generated by Flang with Flags info is 0x0 under RISC-V.

Why does "flang/test/Driver/target-features.f90" list all RISC-V features? Why not use https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/target-cpu-features.f90 instead?

LGTM, thanks!

I'd move these tests to flang/test/Driver/supported-suffices. That would help documenting what these tests actual check for. Also, one could be tempted to test for other suffices and then it would be nice to keep them in one place. Ta!

In D145845#4186608, @clementval wrote:

In D145845#4186607, @awarzynski wrote:

Thanks!

In D145845#4186590, @clementval wrote:

Thanks. Can you add tests in flang/test/Driver?

Hm, this is a clangDriver change rather than anything Flang specific 🤔 . Btw, how does https://github.com/llvm/llvm-project/issues/61260 manifest to you? I'm trying to figure out how to best test it (I couldn't find any other tests for this specifically).

Cannot we just have a a .f03 file and compile it and see if we have an executable?

In D145845#4186590, @clementval wrote:

Thanks. Can you add tests in flang/test/Driver?

Address PR comments

In D145385#4174168, @dcaballe wrote:

I'd love to see this also working for 2D vectors :)

In D144864#4175001, @agozillon wrote:

And then provided @awarzynski is happy with the current state of the patch

Apply clang-format

Thank you very much @awarzynski for suggesting them that was a great help.

LGTM, many thanks for working on this!

[nit]

After this patch:

Refine how contiguous loads are identified

@bkramer , sorry about this issue and thanks for reverting!

Thanks Kiran!

LGTM, thanks!

LGTM, thanks for seeing this through! :)