Page MenuHomePhabricator

[Flang][OpenMP] Implement lowering simd aligned to LLVM IR
Needs RevisionPublic

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

Details

Summary

This patch adds lowering omp simd aligned clause to LLVM IR.

The aligned clause is represented as llvm.assume(i1 true) ["align"(ptr)] instruction in LLVM IR.

Added helper function to MLIR definition of simd loop which determines number of aligned variables

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.

flang/lib/Lower/OpenMP.cpp
1156

Has the patch that added this function been reverted?

Is there are test for this case?

flang/test/Lower/OpenMP/simd.f90
168

We need to increase the test coverage.

171

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.

172–177

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

mlir/test/Target/LLVMIR/openmp-llvm.mlir
762–777

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.Fri, Mar 10, 1:08 AM
flang/lib/Lower/OpenMP.cpp
1156

The patch has been restored: https://reviews.llvm.org/rGbaca3c150733c89686287ba4927c351eec9695e2

Default OpenMP simd alignment is tested here:
https://github.com/llvm/llvm-project/blob/main/clang/test/OpenMP/simd_metadata.c
Would you like to have Flang specific tests as well?

1161–1162

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.

flang/test/Lower/OpenMP/simd.f90
171

I created github issue: https://github.com/llvm/llvm-project/issues/61161 to add this semantic check