Hi Andrew!
On 2019-11-22T11:06:14+0000, Andrew Stubbs <a...@codesourcery.com> wrote:
> This test case causes an ICE (reformatted for email):
>
> void test(int k)
> {
> unsigned int x = 1;
> #pragma acc parallel loop async(x)
> for (int i = 0; i < k; i++) { }
> }Curious, why didn't you include such a test case in your actual patch? > t.c: In function 'test': > t.c:4:9: error: invalid argument to gimple call > 4 | #pragma acc parallel loop async(x) > | ^~~ > (int) x > __builtin_GOACC_parallel_keyed (-1, test._omp_fn.0, 1, > &.omp_data_arr.3, &.omp_data_sizes.4, > &.omp_data_kinds.5, 536936447, > (int) x, 0); > during GIMPLE pass: ompexp > dump file: t.c.013t.ompexp > t.c:4:9: internal compiler error: verify_gimple failed > > The problem is that "x" needs to be cast to "int" (from "unsigned int") > before calling the function, and that's not valid in a gimple call. ACK, thanks. > The attached patch assigns the "(int) x" to a temporary and passes that > to the function instead. > > OK to commit? A little bit more thoroughness, please. :-) As can be seen in the code a few lines below, the very same problem also exists for the 'wait' clause; it seems reasonable to fix both at the same time. This is not a recent regression, but a user-visible valid-code ICE that has existed "forever"; I filed <https://gcc.gnu.org/PR93488> "ICE in type-cast 'async', 'wait' clauses" for tracking. This problem is similar to the OpenMP 'device' clause <https://gcc.gnu.org/PR71758> "ICE in verify_gimple_in_cfg, at tree-cfg.c:5212"; I suggest we also use 'force_gimple_operand_gsi' instead of manually doing your suggested 'create_tmp_var', 'gimple_build_assign', 'gsi_insert_before'. Include a test case that covers all relevant code paths; I've attached a test case to the PR, but I've not verified whether "that covers *all* relevant code paths". This should then be backported to all GCC release branches; I can easily test the backports for you, if you're not already set up to do such testing. Grüße Thomas > Normalize GOACC_parallel_keyed async parameter. > > 2019-11-22 Andrew Stubbs <a...@codesourcery.com> > > gcc/ > * omp-expand.c (expand_omp_target): Pass sync parameter to > GOACC_parallel_keyed via a temporary variable. > > diff --git a/gcc/omp-expand.c b/gcc/omp-expand.c > index 6f945011cf5..08f95587e95 100644 > --- a/gcc/omp-expand.c > +++ b/gcc/omp-expand.c > @@ -8418,7 +8418,12 @@ expand_omp_target (struct omp_region *region) > i_async)); > } > if (t_async) > - args.safe_push (t_async); > + { > + tree tmp = create_tmp_var (TREE_TYPE (t_async)); > + gimple *stmt = gimple_build_assign (tmp, t_async); > + gsi_insert_before (&gsi, stmt, GSI_SAME_STMT); > + args.safe_push (tmp); > + } > > /* Save the argument index, and ... */ > unsigned t_wait_idx = args.length ();
signature.asc
Description: PGP signature
