This is an archive of the discontinued LLVM Phabricator instance.

[flang] Support export/import OpenMP Threadprivate Flag
ClosedPublic

Authored by peixin on Feb 23 2022, 1:23 AM.

Details

Summary

The information about OpenMP/OpenACC declarative directives in modules
should be carried in export mod files. This supports export OpenMP
Threadprivate directive and import OpenMP declarative directives.

Diff Detail

Event Timeline

peixin created this revision.Feb 23 2022, 1:23 AM
peixin requested review of this revision.Feb 23 2022, 1:23 AM
Herald added a project: Restricted Project. · View Herald Transcript

Currently, only OpenMP Threadprivate Flag is resolved in semantic analysis stage. https://github.com/llvm/llvm-project/blob/c1b9672534cdc798f2a7ba6b7b6be85dea0d8a5a/flang/lib/Semantics/resolve-directives.cpp#L483-L484 https://github.com/llvm/llvm-project/blob/c1b9672534cdc798f2a7ba6b7b6be85dea0d8a5a/flang/lib/Semantics/resolve-directives.cpp#L213

What can be in the clause of the declarative directive is case-by-case. For OpenMP Threadprivate directive, the module name, procedure name cannot be in the clause. If a threadprivate directive that specifies a common block name appears in one program unit, then such a directive must also appear in every other program unit that contains a COMMON statement that specifies the same name. So, no need to export common block for Threadprivate directive.

schweitz added inline comments.Feb 23 2022, 8:19 AM
flang/lib/Semantics/mod-file.cpp
328

What are the expectations of having a variable that is both in a COMMON block and flagged as OMP thread private? (This suggests COMMON will have priority over thread private.)

I think you should have a test case below to check that combination of conditions and ensure against regressions, etc.

peixin updated this revision to Diff 410977.Feb 23 2022, 5:14 PM
peixin added inline comments.Feb 23 2022, 5:20 PM
flang/lib/Semantics/mod-file.cpp
328

An element of common block cannot be in Threadprivate directive. The semantic check is in https://github.com/llvm/llvm-project/blob/3e3e79a9e4c378b59f5f393f556e6a84edcd8898/flang/lib/Semantics/check-omp-structure.cpp#L879-L883. The Threadprivate flag is resolved as every element has the flag (https://github.com/llvm/llvm-project/blob/3e3e79a9e4c378b59f5f393f556e6a84edcd8898/flang/lib/Semantics/resolve-directives.cpp#L1640-L1652). No addition information needs to be exported for common block in Threadprivate directive.

Added the test case for common block.

flang/lib/Semantics/mod-file.cpp
328

Would it be better to always generate the declarative directives, irrespective of conditions and then use it conditionally?

peixin updated this revision to Diff 411057.Feb 24 2022, 3:24 AM
peixin added inline comments.Feb 24 2022, 3:30 AM
flang/lib/Semantics/mod-file.cpp
328

Thanks for the notice. I was wrong before. Although an element of common block cannot be in Threadprivate directive, the usage of the element of a Threadprivate common block is as if it is allowed according to the OpenMP Spec 5.0 Section 2.19.4 [Data-sharing attribute clauses]. For example, a list item that appears in a copyin clause must be threadprivate. Named variables that appear in a threadprivate common block may be specified: it is not necessary to specify the whole common block. The specific case is like the following:

$ cat file2.f90
module private_data
	real :: x
	common /blk/ x
    !$OMP THREADPRIVATE(/blk/)
end module private_data
$ cat file1.f90
subroutine a
  use private_data
  !$omp parallel copyin(x)
  !$omp end parallel
end
$ flang-new -fc1 -fopenmp file2.f90 
$ flang-new -fc1 -fopenmp file1.f90

@peixin, LGTM.

The only concern I have is that we will have to do this separately for each declarative construct/directive. If there was something common then it would have worked for everything. But maybe we can get to that when we handle the next declarative directive.

This revision is now accepted and ready to land.Mar 18 2022, 4:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 18 2022, 4:59 AM

But maybe we can get to that when we handle the next declarative directive.

I checked other declarative directives when I implemented this code. The declare target works on both variables and procedure names. The declare reduction works on assignment expression and interface/procedure name. The declare simd works on procedure names. Also, there are some clauses or other infos might should be exported such as initializer in declare reduction. So, each declarative directive will require special semantic analysis and mod file export. This patch can only support variables info export, which is enough for threadprivate directive. Let's add some code or refactor this code when we get to other declarative directives.

@schweitz Is this patch ok to you?

I think you can wait for one more day and submit if there are no further comments.

peixin added a comment.Apr 8 2022, 2:46 AM

I think you can wait for one more day and submit if there are no further comments.

OK. Thanks.

peixin updated this revision to Diff 421674.Apr 8 2022, 8:18 PM

Rebase before land.

This revision was automatically updated to reflect the committed changes.