Hi, > Yeah, the oacc-mem.c is a two liner, we don't need copyright assignment for > that, so please mention Daichi-san's name + mail address, and PR > libgpmp/69414 in the ChangeLog entry.
If possible, could you add " SGI Japan, Ltd." before/after my name? Because I am belonging to the company. Thank you. Regards, Daichi Fukuoka -----Original Message----- From: Jakub Jelinek [mailto:ja...@redhat.com] Sent: Wednesday, March 23, 2016 7:24 PM To: Thomas Schwinge Cc: James Norris; GCC Patches; Fukuoka Daichi Subject: Re: [gomp4] Fix handling of subarrays with update directive On Wed, Mar 23, 2016 at 09:35:30AM +0100, Thomas Schwinge wrote: > Hi! > > On Fri, 22 Jan 2016 12:50:38 -0600, James Norris <jnor...@codesourcery.com> > wrote: > > The attached patch fixes a defect reported with gcc 5.2 > > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69414). > > It is also the case, the issue is present in the gomp4 branch. The > > patch also adds additional testing. > > > > Committed to gomp4 after bootstrap and regtesting. > > Jakub, is that following two-line patch (libgomp/oacc-mem.c, plus test > cases) OK for trunk? > > > --- ChangeLog.gomp (revision 232740) > > +++ ChangeLog.gomp (revision 232741) > > @@ -1,3 +1,11 @@ > > +2016-01-22 James Norris <jnor...@codesourcery.com> > > + > > + * oacc-mem.c (delete_copyout, update_dev_host): Fix device address. > > Given that Daichi Fukuoka submitted the patch in > <https://gcc.gnu.org/PR69414>, please acknowledge that in the ChangeLog. > Please also include the test case submitted there; rename their file > from > gcc1.f90 to pr69414.f90 (or similar). Yeah, the oacc-mem.c is a two liner, we don't need copyright assignment for that, so please mention Daichi-san's name + mail address, and PR libgpmp/69414 in the ChangeLog entry. > > --- oacc-mem.c (revision 232740) > > +++ oacc-mem.c (revision 232741) > > @@ -509,7 +509,7 @@ > > gomp_fatal ("[%p,%d] is not mapped", (void *)h, (int)s); > > } > > > > - d = (void *) (n->tgt->tgt_start + n->tgt_offset); > > + d = (void *) (n->tgt->tgt_start + n->tgt_offset + h - > > + n->host_start); I'd prefer (uintptr_t) h instead of, and you probably need to wrap it then. > > host_size = n->host_end - n->host_start; > > > > @@ -562,7 +562,7 @@ > > gomp_fatal ("[%p,%d] is not mapped", h, (int)s); > > } > > > > - d = (void *) (n->tgt->tgt_start + n->tgt_offset); > > + d = (void *) (n->tgt->tgt_start + n->tgt_offset + h - > > + n->host_start); Ditto. > > --- testsuite/libgomp.oacc-fortran/update-1-2.f90 (revision 0) > > +++ testsuite/libgomp.oacc-fortran/update-1-2.f90 (revision 232741) > > @@ -0,0 +1,239 @@ > > +! Copy of update-1.f90 with self exchanged with host for !$acc > > +update > > Actually, that file contains additional changes. > > Anyway, I suggest we don't add new test cases with just self vs. host > clauses exchanged -- their equivalence is something that should > (already) be tested in GCC front end test cases. I know that we > currently have a distinct libgomp.oacc-c-c++-common/update-1-2.c test > case as a "copy of update-1.c with self exchanged with host for > #pragma acc update", once added by me; I suggest we remove that. > Let's just in the same file use a mixture of host and self clauses. > (I'll take care of that.) Otherwise LGTM, but please repost it with all the testcase changes you want to make. Jakub
