This is an archive of the discontinued LLVM Phabricator instance.

Incorrect implicit data-sharing for nested tasks
ClosedPublic

Authored by smateo on Jan 8 2019, 2:28 AM.

Details

Summary

There is a minor issue in how the implicit data-sharings for nested tasks are computed.

For the following example:

int x;
#pragma omp task shared(x)
#pragma omp task
x++;

We compute an implicit data-sharing of shared for x in the second task although I think that it should be firstprivate. Below you can find the part of the OpenMP spec that covers this example:

  • In a task generating construct, if no default clause is present, a variable for which the data-sharing attribute is not determined by the rules above and that in the enclosing context is determined to be shared by all implicit tasks bound to the current team is shared.
  • In a task generating construct, if no default clause is present, a variable for which the data-sharing attribute is not determined by the rules above is firstprivate.

Since each implicit-task has its own copy of x, we shouldn't apply the first rule.

Diff Detail

Repository
rC Clang

Event Timeline

smateo created this revision.Jan 8 2019, 2:28 AM
smateo updated this revision to Diff 180631.Jan 8 2019, 2:57 AM

Adding more context

ABataev added inline comments.Jan 8 2019, 6:38 AM
lib/Sema/SemaOpenMP.cpp
680

Better to rename it to isImplicitTaskingRegion

smateo updated this revision to Diff 180792.EditedJan 8 2019, 11:00 PM

Renaming the isParallelRegion function to isImplicitTaskingRegion.

Shall we rename the isParallelOrTaskRegionto isImplicitOrExplicitTaskingRegion? It is only called 4 times.

Thanks!

ABataev accepted this revision.Jan 9 2019, 6:14 AM

Renaming the isParallelRegion function to isImplicitTaskingRegion.

Shall we rename the isParallelOrTaskRegionto isImplicitOrExplicitTaskingRegion? It is only called 4 times.

Thanks!

Yes, go ahead and do this.

This revision is now accepted and ready to land.Jan 9 2019, 6:14 AM
smateo updated this revision to Diff 180844.Jan 9 2019, 7:45 AM

Done! Thanks,

Sergi

Will you commit this patch? Or I should do it?

smateo added a comment.Jan 9 2019, 7:52 AM

I don't have commit access yet. Would you mind to commit it for me? Thanks!,

Sergi

I don't have commit access yet. Would you mind to commit it for me? Thanks!,

Sergi

Sure, no problems.

This revision was automatically updated to reflect the committed changes.