This is an archive of the discontinued LLVM Phabricator instance.

[MLIR][Flang][OpenMP] Implement lowering simd aligned to MLIR

Authored by domada on Jan 27 2023, 7:03 AM.



This patch implements lowering simd aligned clause from PFT to MLIR.

Generated MLIR code contains information which MLIR item needs to be aligned. If the user does not specify the alignment value then the default one is calculated on the basis of target features.

Diff Detail

Event Timeline

domada created this revision.Jan 27 2023, 7:03 AM
Herald added a reviewer: ftynse. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
domada requested review of this revision.Jan 27 2023, 7:03 AM
kiranchandramohan requested changes to this revision.Feb 3 2023, 10:08 AM

For this patch, I would recommend moving the translation changes in MLIR into a separate patch.

Please see comments inline.


Has the patch that added this function been reverted?

Is there are test for this case?


We need to increase the test coverage.


From the standard:
Each list item must have C_PTR or Cray pointer type or have the POINTER or ALLOCATABLE attribute. Cray pointer support has been deprecated.

This array does not meet this criteria and hence not a good test. Will also be good to file a github issue to add this semantic check.


PARAM_ARG , ARG_0, ARG_1 etc do not seem to be used. Is there a reason for capturing?

762–777 ↗(On Diff #492742)

Can you minimise this test to what is only needed to check the translation of this clause?

Could you clarify what types are covered and what types are not covered by this change?

This revision now requires changes to proceed.Feb 3 2023, 10:08 AM
domada added inline comments.Mar 10 2023, 1:08 AM

The patch has been restored:

Default OpenMP simd alignment is tested here:
Would you like to have Flang specific tests as well?


I think that we need to modify Flang parser so that the aligned item is modeled as Expr, not as a symbol.

IMO we should firstly modify parser and then update this review.


I created github issue: to add this semantic check

domada planned changes to this revision.Apr 13 2023, 6:31 AM

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


OmpAlignedClause should be modified. It should contain list of Fortran::parser::Variable .


This loop should be modified. We should modify this loop so that it will generate the following MLIR code:

Input Fortran code:

integer, pointer :: a;
! ptr assignment etc.
!$omp simd aligned(a:256)

Pseudo MLIR code which need to be generated:

%a_ptr : !fir.ref<!fir.ref<!fir.ptr<i32>>
; ptr assignment etc.
%a_addr = load %a_ptr
 omp.simdloop aligned(%a_addr -> 256 : i64)

To do it we need to firstly refactor OmpAlignedClause so that it contains list of Fortran::parser::Variable. Then the for loop will look in the following way:

Fortran::lower::StatementContext stmtCtx;
for (const Fortran::parser::Variable &ompItem : ompvarList) {
  alignVars.push_back(fir::getBase(converter.genExprAddr(*Fortran::semantics::GetExpr(ompItem), stmtCtx));
domada edited the summary of this revision. (Show Details)Jul 6 2023, 12:56 AM
domada updated this revision to Diff 537616.Jul 6 2023, 1:02 AM

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

It will be easier if you can split out the MLIR portions into a separate patch. From a quick look, that looks standalone.

domada updated this revision to Diff 548163.Aug 8 2023, 5:22 AM
domada retitled this revision from [Flang][OpenMP] Implement lowering simd aligned to LLVM IR to [MLIR][Flang][OpenMP] Implement lowering simd aligned to MLIR.
domada edited the summary of this revision. (Show Details)

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.

kiranchandramohan accepted this revision.Aug 9 2023, 7:29 AM


Please add couple more tests with:
-> More than one list item in the aligned clause.
-> List item that is a Fortran pointer.
Also check with the gfortran testsuite before you submit.


Nit: No braces.


Nit: Expand auto.


Would passing in a map to getTargetFeatures be better? Will that avoid a copy?


Nit: Can you use getOmpObjectSymbol here, given that Semantics will error out if it is a common block and we will not reach here?


Nit: Does not talk about Less than 0 (which is the comparison used below).

This revision is now accepted and ready to land.Aug 9 2023, 7:29 AM