Hi Michael,
thank you very much for your work.

Patch rebased.
Applied review remarks - scope of the patch is reduced. It contains only the changes which are required to generate MLIR code for simd aligned clause.

Hi,
thank you for your contribution.
Could you fix assertion failure which was reported by Clang CI ( https://buildkite.com/llvm-project/premerge-checks/builds/166519#01898852-f9c9-4dd2-9933-b32be792c976 ) ?
Assertion failed: OpenMPRuntime != nullptr, file C:\ws\w9\llvm-project\premerge-checks\clang\lib\CodeGen\CodeGenModule.h, line 635

LGTM

Patch not needed. Feature already implemented by https://reviews.llvm.org/D153910

Patch rebased. Use OmpObjectList instead of NameList. Added test cases for C_PTR and Fortran pointers.

Patch not needed. Preprocessor change: https://reviews.llvm.org/D153910 enables passing information about original source file.

LGTM

LGTM.
Thank you for your patch.
I tested it, and it solves all issues connected with -save-temps flag, which were mentioned on LLVM discourse.
Could you fix one check statement in flang/test/Parser/line-directive.f90 which fails on Windows?

Hi @raghavendhra
could you also address this issue: https://github.com/llvm/llvm-project/issues/61161 ?

Patch rebased.

Scope of changes:

1. Create one function common for bbc and Flang frontend to set _OPENMP macro
2. Use setOpenMPmacro function inside Flang frontend and bbc source code.

In D151445#4372892, @clementval wrote:

You should be able to get this information from the source location in MLIR.

OpenMP standard mentions that if implementation of OpenMP supports preprocessor then macro _OPENMP is defined to have value yyyymm, where yyyy denotes the year and mm denotes the month of OpenMP version release. (OpenMP standard 5.2 section 3.3).
Currently Flang has hard coded value of OpenMP 4.5 (see line: https://github.com/llvm/llvm-project/blob/main/flang/lib/Frontend/CompilerInvocation.cpp#L994 )
Shall I add information about other versions?

In D150354#4348785, @kiranchandramohan wrote:

In D150354#4342146, @domada wrote:

In D150354#4337148, @awarzynski wrote:

All in all LGTM, but I'm not sure whether Flang should be defaulting to OpenMP 5.0. AFAIK, that's not supported yet.

If you wish I can set to OpenMP 4.5. But then we need to have two separate flags in clang/include/clang/Driver/Options.td (one for clang and the second one for flang).

We will not be able to match the OpenMP support in clang (stand support level) soon. Although we have made lot of progress, we are effectively around 1.1. To stay true to the meaning of this metadata, we will have to have separate flags. But, I guess, fclang-openmp-version might not be acceptable to Clang folks who are used to using -fopenmp-version. Can't this be achieved by the same flag? On a cursory look, i don't see anything that prevents using the same flag but with different defaults in the code that is handling the flag.

Change the default version of OpenMP version flag to 1.1 for flang. Modify the flag description text. No changes for Clang code.

In D150351#4342442, @kiranchandramohan wrote:

LG.

Where will the omp.version be set in Flang? Assuming it is in a separate patch.

In D150354#4337148, @awarzynski wrote:

All in all LGTM, but I'm not sure whether Flang should be defaulting to OpenMP 5.0. AFAIK, that's not supported yet.

Patch rebased because I applied remarks for parent revision.

In D150351#4337358, @kiranchandramohan wrote:

Could you clarify in the commit message that there are two attributes here: One representing the device version (in Flags Attr) and a standalone one called omp.version?

Applied remarks

Patch rebased + clang format

In D150351#4334629, @agozillon wrote:

In D150351#4334624, @kiranchandramohan wrote:

In D150351#4334608, @domada wrote:

In D150351#4334598, @kiranchandramohan wrote:

Will the translation to LLVM be through amendOperation?

That's my plan. If you wish I can wait till the output of @skatrak proposal: https://reviews.llvm.org/D150275

No need to wait. I believe that proposal was only to demonstrate that there is a way for out of tree users to use the Offload Attributes in the Dialect.
Also, if it uses amendOperation then you can consider removing the getVersion since we will not be using it.

Another question I have is whether the flags attribute will always be present with -fopenmp or only with some device/target specific options.

Currently the flags were only present on device (and the original intent was it only being these flags, but it makes sense for it to extend broader to cover these flags as well), but that was because the flags that were in the attribute originally are only ever present on the device/target. However, all you would need to do to change this is move the is_device check that applies them in the frontend (initial attribute creation) to the back end (MLIR attribute -> LLVM IR) and then use the is_device flag in amendOperation to filter and generate the flags you wish. This would mean the attribute is present for host and device whenever -fopenmp is specified however, although I don't necessarily see a problem with that!

Resolve dependency between parent and child revision.

Patch rebased

In D150351#4334598, @kiranchandramohan wrote:

Will the translation to LLVM be through amendOperation?

Fixed default version of OpenMP

In D149450#4326431, @ftynse wrote:

+@ftynse and @mehdi_amini
I believe Alex or Mehdi said before that the passthrough support was just a temporary solution and we must not rely on it in more places.

Indeed. "Passthrough" must not be used unless for quick prototyping to check which attributes are needed before modeling them as properly defined first-class attributes in MLIR. There is no verification, it has strings everywhere. I'll update the documentation to better reflect this.

Patch rebased

Patch rebased

Fortan parser needs to be modified before we will submit lowering simd aligned clause to MLIR.

Updated mlir definition of target attributes. Use MLIR class StringRefParameter instead std::string

Scope of changes:

1. Applied @kiranchandramohan suggestion ( https://reviews.llvm.org/D146721 ) for target interface.
2. Patch rebased.
3. Limited scope of patch. Lowering from MLIR to LLVM IR will be present in different patch

In D145579#4226542, @tschuett wrote:

I wanted to ask whether you want to put an AMDGPU.cpp and AMD.cpp file in the flang/lib/Frontend directory.

Patch rebased and added new test for checking incorrect wavefront sizes AMDGPU target features.

In D145579#4224157, @tschuett wrote:

Do you want to move the AMDGPU changes into AMDGPU.cpp next to AMD.cpp? From the conversation, there seems to be more target specific behaviours.

Patch rebased

Rebase & applied review remarks

Hi Kiran,
thanks for the patch.
It looks good to me and I will update my review: https://reviews.llvm.org/D146612 . I will use interface instead of module attributes.

Added OpenMP target attributes from parent review D145579 .

Applied remarks.
Moved OpenMP changes to https://reviews.llvm.org/D146612 .

D146612 presents the lowering from MLIR attributes to LLVM IR.

Patch rebased

Fixed formatting

In D141910#4112164, @tschuett wrote:

For AArch64 the default alignment is 0? I would have expected 128.