This patch fixes the problem that user-defined mapper array is not correctly privatized inside a task. This problem causes openmp/libomptarget/test/offloading/target_depend_nowait.cpp fails.
I'm not enough invested in the compiler implementation to judge this patch.
I can confirm that this patch fixes the segmentation fault I have seen.
Are these tests supposed to work for offloading to nvidia GPU as well?
I tried and added:
// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
which gives me:
ptxas /tmp/target_depend_nowait-37c5c4.s, line 355; error : Call has wrong number of parameters ptxas /tmp/target_depend_nowait-37c5c4.s, line 628; error : Call has wrong number of parameters ptxas /tmp/target_depend_nowait-37c5c4.s, line 2331; warning : Instruction 'vote' without '.sync' may produce unpredictable results on sm_70 and later architectures ptxas /tmp/target_depend_nowait-37c5c4.s, line 2331; warning : Instruction 'vote' without '.sync' is deprecated since PTX ISA version 6.0 and will be discontinued in a future PTX ISA version ptxas fatal : Ptx assembly aborted due to errors
I guess this error is not directly related to your patch?
Does this mean, that a task is only created if depend clauses are provided?
The problem you had is probably not related to this patch. A lot of libomptarget tests are not working for nvptx now. I suppose someone is going to fix them.
The case you describe, which is to have a task wrapped around target, is solved with modification on line 9592
Do you really need this IsTask flag? The array is created always when Info.NumberOfPtrs > 0. Maybe, just do not check for HasMapper here?
Better to add a flag RequiresOuterTask = D.hasClausesOfKind<OMPDependClause>() and use it here and where the task is generated (line 10455)
This is an optimization. The mapper array can be optimized out when there is no actual mappers, so supposed to have less memory accesses in this common case. Not checking HasMapper will disable this optimization.
Sure, please accept this patch after that.
i applied this patch to our downstream build for amdgcn. it resolves the problem i was seeing with traps on tests with task wait depend tests from sollve.
perhaps rebase it to latest llvm.
[AMD Official Use Only - Internal Distribution Only]
Any ETA on this one ?
Get Outlook for iOShttps://aka.ms/o0ukef
From: Alexey Bataev via Phabricator <firstname.lastname@example.org>
Sent: Tuesday, September 8, 2020 1:39:06 PM
To: email@example.com <firstname.lastname@example.org>; email@example.com <firstname.lastname@example.org>; email@example.com <firstname.lastname@example.org>; email@example.com <firstname.lastname@example.org>
Cc: Lieberman, Ron <Ron.Lieberman@amd.com>; email@example.com <firstname.lastname@example.org>; email@example.com <firstname.lastname@example.org>; Liu, Yaxun (Sam) <Yaxun.Liu@amd.com>; email@example.com <firstname.lastname@example.org>; email@example.com <firstname.lastname@example.org>
Subject: [PATCH] D84470: [OpenMP 5.0] Fix user-defined mapper privatization in tasks
[CAUTION: External Email]
ABataev added a comment.
I can update this patch in a week when I'm back from vacation.
CHANGES SINCE LAST ACTION