This is an archive of the discontinued LLVM Phabricator instance.

[Flang][OpenMP] Initial lowering of the OpenMP worksharing loop
ClosedPublic

Authored by kiranchandramohan on May 5 2022, 10:42 AM.

Details

Summary

The OpenMP worksharing loop operation in the dialect is a proper loop
operation and not a container of a loop. So we have to lower the
parse-tree OpenMP loop construct and the do-loop inside the construct
to a omp.wsloop operation and there should not be a fir.do_loop inside
it. This is achieved by skipping fir.do_loop creation and calling genFIR
for the nested evaluations in the lowering of the do construct.

Note: Handling of more clauses, parallel do, storage of loop index variable etc will come in separate patches.

Part of the upstreaming effort to move LLVM Flang from fir-dev branch of
https://github.com/flang-compiler/f18-llvm-project to the LLVM Project.

Co-authored-by: Sourabh Singh Tomar <SourabhSingh.Tomar@amd.com>
Co-authored-by: Shraiysh Vaishay <Shraiysh.Vaishay@amd.com>

Diff Detail

Event Timeline

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 5 2022, 10:42 AM
kiranchandramohan requested review of this revision.May 5 2022, 10:42 AM
kiranchandramohan edited the summary of this revision. (Show Details)May 5 2022, 11:20 AM
kiranchandramohan added a reviewer: clementval.
peixin accepted this revision.May 5 2022, 7:42 PM

LGTM.

Will @Leporacanthicus upstream the collapse and schedule chunk clauses? I will upstream the ordered clause and fix that bug in fir-dev.

flang/lib/Lower/Bridge.cpp
2251

Why do you remove parser::NamelistStmt?

flang/test/Lower/OpenMP/omp-wsloop.f90
4

Since we won't add integration tests under flang/test/Lower/OpenMP, we don't need to differentiate the FIRDialect and OMPDialect, and LLVMIRDialect anymore, and can use the FileCheck without the prefix for future test cases to save our time, right?

This revision is now accepted and ready to land.May 5 2022, 7:42 PM
kiranchandramohan marked an inline comment as done.

Address review comments from @peixin.

kiranchandramohan marked an inline comment as done.May 6 2022, 3:16 AM
kiranchandramohan added inline comments.
flang/lib/Lower/Bridge.cpp
2251

This was an accident. Thanks.

flang/test/Lower/OpenMP/omp-wsloop.f90
4

Changed to CHECK lines.

BTW, I remembered that we have the following file for FIR&OpenMP -> LLVM dialect tests. We should add a test there for all constructs. Since there is already an entry for wsloop, not adding in this case.
https://github.com/llvm/llvm-project/blob/main/flang/test/Fir/convert-to-llvm-openmp-and-fir.fir

This revision was automatically updated to reflect the committed changes.
kiranchandramohan marked an inline comment as done.
peixin added inline comments.May 6 2022, 7:32 AM
flang/test/Lower/OpenMP/omp-wsloop.f90
4

Right. I have tested the execution of some combinations of clauses for worksharing-loop in fir-dev before, even though I didn't test all combinations. The worksharing-loop with some basic support should be OK. It's no hurry to add those test cases. When all upstreaming work is done, one WIP patch adding all the constructs in convert-to-llvm-openmp-and-fir.fir is fine. Maybe let's finish the upstreaming work first.

BTW, I will add the test cases for atomic read/write including memref type conversion when updating D124610.