[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a subscriber: fghanim.
jdoerfert added a comment.

In D75591#1916433 <https://reviews.llvm.org/D75591#1916433>, @atmnpatel wrote:

> Modified the unit test for CodeGen of default(firstprivate) to more accurately reflect the IR.
>
> From the perspective of the AST, the variables that are firstprivate from a default clause are managed in a way that is similar to how variables in a task are marked firstprivate. This is responsible for the discrepancy between the IR for 'default(firstprivate)' and 'default(none) firstprivate(...)'. When a firstprivate clause is handled, it has an explicit list where it stores all private copies that it has constructed for each variable because it has access to an explicit list of variables for which it pre-emptively creates new copies for. For a default clause, if a new copy of a variable needs to be created in a manner idential to 'firstprivate(...)', it would require a complete restructuring of how the default clause is handled because before the constructor for the default clause can be called, we would need to iterate over the contents of the region in order to find references to variables that would need to become firstprivate to create and store copies.
>
> The way that I see it, is that this would be require a much more substantial change (than the current proposed implementation) to the handling of the default clause - the construction of the default clause node in the AST would need to be postponed until after the contents of the openmp region had been placed into an AST and the nodes visited in order to find the relevant variables. It is my belief that this would lead tto a more convoluted implementation whose behavior is identical to the current one.
>
> I can confirm that in the IR it does not behave as an explicit firstprivate clause for the variables it marks as firstprivate but I don't understand enough to know why this causes a functional difference because the variables are handled as firstprivate in the same way that task handles them.


I think we can make `default(firsprivate)` and `default(private)` work for the OpenMPIRBuilder path much easier. I was hoping we could get it before but I get the complications you describe.

@fghanim where are we with the privatization in the new codegen?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75591/new/

https://reviews.llvm.org/D75591