This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: gomp-nvptx branch - middle-end changes
On Tue, Nov 22, 2016 at 08:25:45PM +0300, Alexander Monakov wrote:
> On Fri, 11 Nov 2016, Jakub Jelinek wrote:
> > Ok for trunk, once the needed corresponding config/nvptx bits are committed,
> > with one nit below that needs immediate action and the rest can be resolved
> > incrementally. I'd like to check in afterwards the attached patch, at least
> > for now, so that non-offloaded SIMD code is less affected.
>
> Testing your patch revealed an issue in Fortran offloaded code; types of
> boolean_type_node in f951 and boolean_false_node in lto1 (when omp_device_lower
> runs) don't match. I'm attaching a revised patch that addresses it by simply
> using an integer type (there are also two other minor issues, below).
Ok.
> > Please change this into
> > (ENABLE_OFFLOADING && (flag_openmp || in_lto))
> > for now, so that we don't waste compile time even when clearly it
> > isn't needed, and incrementally change the inliner to propagate
> > the property.
>
> As ENABLE_OFFLOADING is not set in the offloading compiler, this additionally
> needs to accept ACCEL_COMPILER. Applied like this:
>
> + virtual bool gate (function *ARG_UNUSED (fun))
> + {
> + /* FIXME: this should use PROP_gimple_lomp_dev. */
> +#ifdef ACCEL_COMPILER
> + return true;
> +#else
> + return ENABLE_OFFLOADING && (flag_openmp || in_lto_p);
> +#endif
> + }
Makes sense.
> > @@ -4314,6 +4364,12 @@ lower_rec_simd_input_clauses (tree new_v
> > if (max_vf == 0)
> > {
> > max_vf = omp_max_vf ();
> > + if (find_omp_clause (gimple_omp_for_clauses (ctx->stmt),
> > + OMP_CLAUSE__SIMT_))
> > + {
> > + int max_simt = omp_max_simt_vf ();
> > + max_vf = MAX (max_vf, max_simt);
> > + }
>
> I don't believe here there's a need to take a maximum. Cloning the loop upfront
> means that SIMD+SIMT styles are not going to mix within a single loop. I've
> simplified it to an if-then-else in the revised patch.
Ok.
> > @@ -10601,7 +10656,11 @@ expand_omp_simd (struct omp_region *regi
> > bool offloaded = cgraph_node::get (current_function_decl)->offloadable;
> > for (struct omp_region *rgn = region; !offloaded && rgn; rgn = rgn->outer)
> > offloaded = rgn->type == GIMPLE_OMP_TARGET;
> > - bool is_simt = offloaded && omp_max_simt_vf () > 1 && safelen_int > 1;
> > + bool is_simt
> > + = (offloaded
> > + && find_omp_clause (gimple_omp_for_clauses (fd->for_stmt),
> > + OMP_CLAUSE__SIMT_)
> > + && safelen_int > 1);
>
> Here computation of 'offloaded' is no longer needed, because presence of
> OMP_CLAUSE__SIMT_ would imply that. Removed in the revised patch.
>
> I've noticed that your patch doesn't adjust 'maybe_simt' in "ordered" lowering.
> Not sure if that's intentional -- as I understand it's possible to look at the
> enclosing context's clauses because 'omp ordered' must be closely nested with
Right now omp ordered simd for non-simt basically causes vf 1, because the
vectorizer isn't ready for having non-vectorized portions of code within
vectorized loop.
> the corresponding loop. I've added a FIXME in the patch.
Ok for trunk, thanks.
Jakub