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
