Re: [PATCH, 2/3, OpenMP] Target mapping changes for OpenMP 5.0, middle-end parts and compiler testcases

[Jakub Jelinek <jakub@redhat.com>]

On Wed, Oct 28, 2020 at 06:32:21PM +0800, Chung-Lin Tang wrote:
> > > @@ -8958,25 +9083,20 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p,
> > >   	      /* An "attach/detach" operation on an update directive should
> > >   		 behave as a GOMP_MAP_ALWAYS_POINTER.  Beware that
> > >   		 unlike attach or detach map kinds, GOMP_MAP_ALWAYS_POINTER
> > >   		 depends on the previous mapping.  */
> > >   	      if (code == OACC_UPDATE
> > >   		  && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_ATTACH_DETACH)
> > >   		OMP_CLAUSE_SET_MAP_KIND (c, GOMP_MAP_ALWAYS_POINTER);
> > > -	      if (gimplify_expr (pd, pre_p, NULL, is_gimple_lvalue, fb_lvalue)
> > > -		  == GS_ERROR)
> > > -		{
> > > -		  remove = true;
> > > -		  break;
> > > -		}
> > So what gimplifies those now?
> 
> They're gimplified somewhere during omp-low now.
> (some gimplify scan testcases were adjusted to accommodate this change)
> 
> I don't remember the exact case I encountered, but there were some issues with gimplified
> expressions inside the map clauses making some later checking more difficult. I haven't seen
> any negative effect of this modification so far.

I don't like that, it goes against many principles, gimplification really
shouldn't leave around non-GIMPLE IL.
If you need to compare same expression or same expression bases later,
perhaps detect the equalities during gimplification before actually gimplifying the
clauses and ensure they are gimplified to the same expression or are using
same base (e.g. by adding SAVE_EXPRs or TARGET_EXPRs before the
gimplification).

	Jakub