[PATCH 1/5] omp-low: introduce omplow_simd_context

Jakub Jelinek jakub@redhat.com
Wed Jan 18 15:11:00 GMT 2017


On Wed, Jan 18, 2017 at 06:02:06PM +0300, Alexander Monakov wrote:
> > > +struct omplow_simd_context {
> > > +  tree idx;
> > > +  tree lane;
> > > +  int max_vf;
> > > +  bool is_simt;
> > 
> > Any reason ivar and lvar weren't added there too?
> 
> Yes, they are not part of 'persistently live context' between the two functions.
> ivar and lvar are built from scratch in the callee, and immediately used only
> once in the caller.

Ok.

> > > +  int &max_vf = sctx->max_vf;
> > >    if (max_vf == 0)
> > >      {
> > > -      if (omp_find_clause (gimple_omp_for_clauses (ctx->stmt),
> > > -			   OMP_CLAUSE__SIMT_))
> > > -	max_vf = omp_max_simt_vf ();
> > > -      else
> > > -	max_vf = omp_max_vf ();
> > > +      max_vf = sctx->is_simt ? omp_max_simt_vf () : omp_max_vf ();
> > 
> > I think it would be better just to use sctx->max_vf instead of max_vf,
> > without the reference.
> 
> There are 5 more references to max_vf below; I've introduced the reference to
> keep the patch smaller, but I think it improves readability too.  But I won't
> mind changing it to sctx->max_vf everywhere.

I think I like sctx->max_vf better, it is more consistent with the other
field accesses, makes it more obvious what you store into etc.

	Jakub



More information about the Gcc-patches mailing list