Skip to content

[flang][runtime] Add a critical section for LookUpOrCreateAnonymous. #74468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 6, 2023
Merged

[flang][runtime] Add a critical section for LookUpOrCreateAnonymous. #74468

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c8022c
[flang][runtime] Ensure that the anonymous unit returned by LookUpOrC…
sihuan Dec 5, 2023
78269ad
fix typo
sihuan Dec 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions flang/runtime/unit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace Fortran::runtime::io {
// The per-unit data structures are created on demand so that Fortran I/O
// should work without a Fortran main program.
static Lock unitMapLock;
static Lock createOpenLock;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This lock can be declared static in the only function that uses it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I move this declaration into *ExternalFileUnit::LookUpOrCreateAnonymous, like the following:

ExternalFileUnit *ExternalFileUnit::LookUpOrCreateAnonymous(int unit,
    Direction dir, std::optional<bool> isUnformatted,
    const Terminator &terminator) {
  // Make sure that the returned anonymous unit has been opened
  // not just created in the unitMap.
  static Lock createOpenLock;
  CriticalSection critical{createOpenLock};
  bool exists{false};
  ExternalFileUnit *result{
      GetUnitMap().LookUpOrCreate(unit, terminator, exists)};
  if (result && !exists) {
    IoErrorHandler handler{terminator};
    result->OpenAnonymousUnit(
        dir == Direction::Input ? OpenStatus::Unknown : OpenStatus::Replace,
        Action::ReadWrite, Position::Rewind, Convert::Unknown, handler);
    result->isUnformatted = isUnformatted;
  }
  return result;
}

I will have to add the -lstdc++ flag when compiling with flang, or I will get a linker error:

$ cat test.f90
! test.f90
!$omp parallel
write(1,*) 'OK'
!$omp end parallel
end
$ ../../llvm-project/build/bin/flang-new test.f90 -fopenmp
/usr/bin/ld: /path/to/llvm-project/build/lib/libFortranRuntime.a(unit.cpp.o): in function `Fortran::runtime::io::ExternalFileUnit::LookUpOrCreateAnonymous(int, Fortran::runtime::io::Direction, std::optional<bool>, Fortran::runtime::Terminator const&)':
/path/to/llvm-project/flang/runtime/unit.cpp:57:(.text+0x32e): undefined reference to `__cxa_guard_acquire'
/usr/bin/ld: /path/to/llvm-project/flang/runtime/unit.cpp:57:(.text+0x369): undefined reference to `__cxa_guard_release'
flang-new: error: linker command failed with exit code 1 (use -v to see invocation)
$ ../../llvm-project/build/bin/flang-new test.f90 -fopenmp -lstdc++
$ OMP_NUM_THREADS=4 ./a.out  
$ cat fort.1
 OK
 OK
 OK

static UnitMap *unitMap{nullptr};
static ExternalFileUnit *defaultInput{nullptr}; // unit 5
static ExternalFileUnit *defaultOutput{nullptr}; // unit 6
Expand Down Expand Up @@ -52,6 +53,9 @@ ExternalFileUnit *ExternalFileUnit::LookUpOrCreate(
ExternalFileUnit *ExternalFileUnit::LookUpOrCreateAnonymous(int unit,
Direction dir, std::optional<bool> isUnformatted,
const Terminator &terminator) {
// Make sure that the returned anonymous unit has been opened
// not just created in the unitMap.
CriticalSection critical{createOpenLock};
bool exists{false};
ExternalFileUnit *result{
GetUnitMap().LookUpOrCreate(unit, terminator, exists)};
Expand Down