Re: [Patch, v2] OpenMP/Fortran: Fix handling of 'declare target' with 'link' clause [PR11555]

[Andre Vehreschild <vehre@gmx.de>]

Hi Tobias,

I am wondering why the testcase has no `!{ dg-do ... }` line. What will dejagnu
do then? Sorry for the may be stupid question, but I never encountered a
testcase without a dg-do line. It was the minimum for me.

Besides that the patch looks ok to me. 

- Andre

On Fri, 26 Jul 2024 20:34:18 +0200
Tobias Burnus <tburnus@baylibre.com> wrote:

> Updated patch - only change is to the testcase:
> 
> * With the just posted patch for PR116107, array sections with offset 
> work for 'link', hence, I updated the testcase.
> 
> * For 'arr2', I added ref to the associated PR.
> 
> I intent to commit it once PR116107 has been committed.
> 
> Tobias
> 
> Tobias Burnus wrote:
> > Hi all,
> >
> > it turned out that 'declare target' with 'link' clause was broken in
> > multiple ways.
> >
> > The main fix is the attached patch, i.e. namely pushing the variables
> > already to the offload-vars list already in the FE.
> >
> > When implementing it, I noticed:
> > * C has a similar issue when using nested functions, which is
> >    a GNU extension →https://gcc.gnu.org/115574
> >
> > * When doing partial mapping of arrays (which is one of the reasons for
> > 'link'), offsets are mishandled in Fortran (not tested in C), see FIXME in
> > the patch) There: arr2(10) should print 10 but with map(arr2(10:)) it
> > prints 19. (I will file a PR about this).
> >
> > * It might happen that linked variables do not get linked. I have not
> > investigated why, but 'arr2' gives link errors – while 'arr' works.
> >    See FIXME in the patch. (I will file a PR about this)
> >
> > * For COMMON blocks, map(/common/) is rejected,https://gcc.gnu.org/PR115577
> >
> > * When then mapping map(a,b,c) which is identical for 'common /mycom/
> > a,b,c', it fails to link the device side as the 'mycom_' symbol cannot be
> > found on the device side.  (I will file a PR about this)
> >
> > As COMMON as issues, an alternative would be to defer the trans-common.cc
> > changes to a later patch.
> >
> > Comments, questions, concerns?
> >
> > Tobias
> >
> > PS: Tested with nvptx offloading with a page-migration supporting system
> > with nvptx and GCN offloading configured and no new fails observed.  


-- 
Andre Vehreschild * Email: vehre ad gmx dot de