This is an archive of the discontinued LLVM Phabricator instance.

[flang] Fix host associated vars in OpenMP/OpenACC region
Needs ReviewPublic

Authored by peixin on Mar 9 2023, 4:16 AM.

Details

Summary

When making the symbols of host associated vars and the host is used
associated, the scope is not checked if it is the subprogram or the
OpenMP/OpenACC construct. As a result, the symbols are not added into
the subprogram scope, and they are not instantiated in lowering. Add
OMPACCConstruct scope kind to check this case.

Fix #60739.

Diff Detail

Event Timeline

peixin created this revision.Mar 9 2023, 4:16 AM
Herald added a project: Restricted Project. · View Herald Transcript
peixin requested review of this revision.Mar 9 2023, 4:16 AM

@peixin Thanks for working on this. Could you check why the patch application is failing?

peixin updated this revision to Diff 503986.Mar 9 2023, 5:08 PM

Thanks @kiranchandramohan for the notice. Rebase to fix patch application fail.

Thanks @peixin for the patch.

I was wondering where is the right place to fix this issue. I see that this works correctly when a block construct is present with handling in Lowering. A modified version of one of your tests would be,

module tpmod
  real :: val = 2.0
end module

subroutine tptest
  use tpmod
  call tps()
contains
  subroutine tps()
    !$omp parallel
    block
      val = 1.0
    end block
    !$omp end parallel
  end subroutine
end subroutine

Would calling instantiateVar for the scope variables as is done for block (https://github.com/llvm/llvm-project/blob/2d68a42f084a460007b368eab191cf0ff1b976d7/flang/lib/Lower/Bridge.cpp#L2282) help fix the issue? Or did you try this and face any issues?

Thanks @peixin for the patch.

I was wondering where is the right place to fix this issue. I see that this works correctly when a block construct is present with handling in Lowering. A modified version of one of your tests would be,

module tpmod
  real :: val = 2.0
end module

subroutine tptest
  use tpmod
  call tps()
contains
  subroutine tps()
    !$omp parallel
    block
      val = 1.0
    end block
    !$omp end parallel
  end subroutine
end subroutine

Would calling instantiateVar for the scope variables as is done for block (https://github.com/llvm/llvm-project/blob/2d68a42f084a460007b368eab191cf0ff1b976d7/flang/lib/Lower/Bridge.cpp#L2282) help fix the issue? Or did you try this and face any issues?

The block construct can contain declarations, so it is reasonable to maintain the variables instantiation in the BLOCK region. However, for OpenMP constructs, one problem is that we assume the variables scope is one level up for privatized clauses and threadprivate variables. The other problem, I think, is to make the design of the variables scope not clean.

peixin added a comment.Jul 7 2023, 8:51 AM

@kiranchandramohan Hi Kiran, could you please help commandeer this patch? I cannot continue working on it due to work changes.