Re: [gomp] Add langhook, so that Fortran can privatize variables by reference

[Thomas Schwinge <thomas@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 2303 bytes --]

Hi Jakub!

On Mon, 27 May 2019 18:49:20 +0200, Jakub Jelinek <jakub@redhat.com> wrote:
> On Sun, May 26, 2019 at 07:43:04PM +0200, Thomas Schwinge wrote:
> > On Tue, 18 Oct 2005 03:01:40 -0400, Jakub Jelinek <jakub@redhat.com> wrote:
> > > --- gcc/omp-low.c.jj	2005-10-15 12:00:06.000000000 +0200
> > > +++ gcc/omp-low.c	2005-10-18 08:46:23.000000000 +0200
> > > @@ -126,7 +126,7 @@ is_variable_sized (tree expr)
> > >  static inline bool
> > >  is_reference (tree decl)
> > >  {
> > > -  return TREE_CODE (TREE_TYPE (decl)) == REFERENCE_TYPE;
> > > +  return lang_hooks.decls.omp_privatize_by_reference (decl);
> > >  }
> > 
> > With the same implementation, this function nowadays is known as
> > 'omp_is_reference' ('gcc/omp-general.c'), and is used in 'omp-*' files
> > only.  The gimplifier directly calls
> > 'lang_hooks.decls.omp_privatize_by_reference'.
> > 
> > Will it be OK to commit the obvious patch to get rid of the
> > 'omp_is_reference' function?  Whenever I see it used in 'omp-*' files, I
> 
> No, omp_is_reference (something) is certainly more readable from
> lang_hooks.decls.omp_privatize_by_reference (something)

Yes, better readable because it's shorter, but you have to look up its
meaning, whereas with 'lang_hooks.decls.omp_privatize_by_reference' you
directly see what it's about.

> which is quite
> long and would cause major issues in formatting etc.

Well, we have rules about how to deal with the formatting issues.

> What advantage do you see in removing that?

For me, it's confusing, when looking at, say, 'OMP_CLAUSE_FIRSTPRIVATE'
code, that in 'gcc/gimplify.c' we call
'lang_hooks.decls.omp_privatize_by_reference', whereas in 'gcc/omp-*.c'
files we call 'omp_is_reference' -- but both actually mean the same
thing.

> > wonder and have to look up what special things it might be doing -- but
> > it actually isn't.
> > 
> > 	gcc/
> >         * omp-general.c (omp_is_reference): Don't define.  Adjust all users.

Or, of course, the other way round:

	gcc/
        * gimplify.c: Use omp_is_reference.

Or, even more preferably:

	gcc/
	* omp-general.c (omp_is_reference): Rename to...
        (omp_privatize_by_reference): ... this.  Adjust all users.
        * gimplify.c: Use it.


Grüße
 Thomas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]