Looks like flang tests failed. Is flang broken?

I don't agree that this new dialect should extend FIR. They should be separate dialects.

I wasn't objecting. I was informing. (Flang did not even have a driver for a long, long time.)

For what it is worth, flang was originally written to be strict rather than permissive. Checks are made to encode things like "this configuration has been tested" rather than just letting everything fall through and sifting through the fallout after the fact. Broadly speaking, the intention was for the compiler to produce known correct output/code when it could. If something was untested, unsupported, unimplemented, etc. however, the compiler was to give a reasonable error message rather than a possibly false impression that everything might have been ok or not.

In D133820#3789710, @vzakhari wrote:

In D133820#3789150, @Leporacanthicus wrote:

Again, thanks for the work here. It looks roughly like I had imagined [although I was kind of wishfully thinking that maybe we could just treat ND as 1D, just larger - at least for things where "ordering isn't important", such as sum or maxval]

If you mean that we could have used a single loop vs the loop nest, then I do not think it is easily possible. fir.coordinate_of provided with the multiple indices accounts for the dimension strides (which can be anything in case of slicing) - that we would have to do manually if using a single loop. Using the loop nest looks much cleaner to me and consistent with the rest of FIR generation.

What is the motivating Fortran example here?

LGTM. (The bots are complaining.)

Kiran makes a good point that there is a strong desire to perform a true mem-to-reg translation on FIR which would eliminate dead stores and register promote memory traffic to stack slots, etc.

In D125632#3543959, @peixin wrote:

@jeanPerier @schweitz Sorry to bother you again for this issue.

Previously, I mainly tested gfortran and ifort, and did not check classic-flang too much. I checked classic-flang for this issue. It does not support this, either. We tested a lot of workloads using classic-flang and did not get the error for this. So I take it as edge case, and plan not to work on recently.

I tested a lot of F95 test cases. The good news is LLVM Flang in fir-dev performs kind of better than gfortran and classic-flang except for some TODOs. Next, I will include the tests for derived type (including PDT), interoperability with C, and some simple F2003 features. Please let me know if there are some features you cares more and I am happy to make some checks and tests.

Thanks for adding the tests.

In D124610#3543030, @shraiysh wrote:

In D124610#3542998, @schweitz wrote:

In D124610#3540106, @shraiysh wrote:

Yes, this looks good to me for the time being as memref isn't supported. Support for memref can be added later.

You'll want to be careful. The MLIR memref type, when lowered to LLVM, is incompatible with Fortran descriptors and as such might cause some grief. (In FIR, it is a requirement to manage memory references in a way that supports varied Fortran calling conventions.)

IIUC, the concern arises when FIR Dialect types are used in the same IR file as MemRef types. There should be no problem having a memref lowering independent of FIR, right? i.e. Both (OpenMP + Memref -> LLVM) and (OpenMP + FIR -> LLVM) should not be tricky but (OpenMP + FIR + Memref -> LLVM) could be tricky. Is that correct?

In D124610#3540106, @shraiysh wrote:

Yes, this looks good to me for the time being as memref isn't supported. Support for memref can be added later.

array_modify is deprecated. It is no longer needed.

Fixes clang-format issues on the entire file.

LGTM. It's consistent with the overall project and will be easier for the community to maintain in the long run.

If you want to write a Fortran with OpenMP test that tests more of the pipeline, say to LLVM IR, then flang/test/Driver seems to be one possibility of where to put such a test. That directory is not just driver tests, but more a sundry of tests.

Fix clang-format issue.

In D124277#3474801, @clementval wrote:

In D124277#3474792, @awarzynski wrote:

I have changed the name to do_loop.f90 and will write tests in separate files.

I'm not against making test clearer to read but this is again changes that will make the end of upstreaming harder.

I don't quite see how. This was is not present in fir-dev anyway. Also, since we are near the end, perhaps it's OK to be a bit more relaxed?

These tests were present in fir-dev in their initial form. Near the end is exactly where it becomes harder to be sure everything is upstreamed if there is lots of noise in the diffs.

In D124277#3472480, @awarzynski wrote:

It's great to see do loop finally making its way into LLVM main, thanks for working on this @kiranchandramohan !

I would really appreciate if you could break the tests into smaller units. Currently, it's quite tricky to follow them. Indeed, do loop itself is quite complex and I think that it would be beneficial to keep the tests to the required minimum. For example, loop_test1 could be split into:

• a test for a most basic do loop (i.e. just the loop),
• a test for a nested do loop,
• a test for do loop with accumulation that depends on the index variable.

I think that 2 nested loops should be enough, unless we know that things change when moving from 2 to 3 nested loops. Also:

• is print ing really required in these tests? (that's adding complexity that's IMHO not really needed here)
• why not use more descriptive names for tests?

Thank you!

It doesn't look familiar to me, @kiranchandramohan .

In D122725#3461702, @kiranchandramohan wrote:

LGTM. Please wait for approval from @clementval/@schweitz for the location of attaching the OpenMP interface and any outstanding comments from the other reviewers.

Good to see these improvements going in to MLIR's CSE, Valentin.

In D121171#3429309, @awarzynski wrote:

We pledged to retire bbc/tco in favour of flang-new/fir-opt and this change is a step in the opposite direction (at least in it's current format) . I think that we should maintain the interoperability of tests using bbc/tco and flang-new.

Thanks for upstreaming these, Valentin.

LGTM.

Thanks for upstreaming these, Andrzej.

Note: The test should be extended to exercise the elemental aspect of IOR when that gets upstreamed as well.