Hi Jakub! On Wed, 19 Dec 2018 15:18:12 +0100, Jakub Jelinek wrote: > On Wed, Dec 19, 2018 at 03:03:42PM +0100, Jakub Jelinek wrote: > > On Wed, Dec 19, 2018 at 02:59:54PM +0100, Thomas Schwinge wrote: > > > Right. For OpenACC, there's no "device" clause, so we only ever passed > > > in "GOMP_DEVICE_ICV" (default), or "GOMP_DEVICE_HOST_FALLBACK" ("if > > > (false)" clause). Therefore, the libgomp "resolve_legacy_flags" function > > > added to make sure that these two values (as used by old executables) > > > continue to work as before (with new libgomp). (And, we have to make > > > sure that no (new) "GOACC_FLAG_*" combination ever results in these > > > values; will document that.) > > LGTM then in principle. > > Or keep it int and use inverted bitmask, thus when bit is 1, it represents > the default state and when bit is 0, it is something different from it. Ha, I too had that idea after thinking some more about the "-1" and "-2" values/representations... :-) > If you passed before just -1 and -2 and because we are only supporting two's > complement, the host fallback test would be (flags & 1) == 0. I structured that a bit more conveniently. That's especially useful once additional flags added, where you want to just do "flags |= [flag]", etc. > Then you don't need to at runtime transform from legacy to non-legacy. Right. Is the attached OK for trunk? If approving this patch, please respond with "Reviewed-by: NAME " so that your effort will be recorded in the commit log, see . For your review convenience, here's the "gcc/omp-expand.c" changes with "--ignore-space-change" (as I slightly restructured OpenACC vs. OpenMP code paths): @@ -7536,49 +7536,62 @@ expand_omp_target (struct omp_region *region) clauses = gimple_omp_target_clauses (entry_stmt); - /* By default, the value of DEVICE is GOMP_DEVICE_ICV (let runtime - library choose) and there is no conditional. */ - cond = NULL_TREE; - device = build_int_cst (integer_type_node, GOMP_DEVICE_ICV); - - c = omp_find_clause (clauses, OMP_CLAUSE_IF); - if (c) - cond = OMP_CLAUSE_IF_EXPR (c); - + device = NULL_TREE; + tree goacc_flags = NULL_TREE; + if (is_gimple_omp_oacc (entry_stmt)) + { + /* By default, no GOACC_FLAGs are set. */ + goacc_flags = integer_zero_node; + } + else + { c = omp_find_clause (clauses, OMP_CLAUSE_DEVICE); if (c) { - /* Even if we pass it to all library function calls, it is currently only - defined/used for the OpenMP target ones. */ - gcc_checking_assert (start_ix == BUILT_IN_GOMP_TARGET - || start_ix == BUILT_IN_GOMP_TARGET_DATA - || start_ix == BUILT_IN_GOMP_TARGET_UPDATE - || start_ix == BUILT_IN_GOMP_TARGET_ENTER_EXIT_DATA); - device = OMP_CLAUSE_DEVICE_ID (c); clause_loc = OMP_CLAUSE_LOCATION (c); } else + { + /* By default, the value of DEVICE is GOMP_DEVICE_ICV (let runtime + library choose). */ + device = build_int_cst (integer_type_node, GOMP_DEVICE_ICV); clause_loc = gimple_location (entry_stmt); + } c = omp_find_clause (clauses, OMP_CLAUSE_NOWAIT); if (c) flags_i |= GOMP_TARGET_FLAG_NOWAIT; + } + /* By default, there is no conditional. */ + cond = NULL_TREE; + c = omp_find_clause (clauses, OMP_CLAUSE_IF); + if (c) + cond = OMP_CLAUSE_IF_EXPR (c); + /* If we found the clause 'if (cond)', build: + OpenACC: goacc_flags = (cond ? goacc_flags : flags | GOACC_FLAG_HOST_FALLBACK) + OpenMP: device = (cond ? device : GOMP_DEVICE_HOST_FALLBACK) */ + if (cond) + { + tree *tp; + if (is_gimple_omp_oacc (entry_stmt)) + tp = &goacc_flags; + else + { /* Ensure 'device' is of the correct type. */ device = fold_convert_loc (clause_loc, integer_type_node, device); - /* If we found the clause 'if (cond)', build - (cond ? device : GOMP_DEVICE_HOST_FALLBACK). */ - if (cond) - { + tp = &device; + } + cond = gimple_boolify (cond); basic_block cond_bb, then_bb, else_bb; edge e; tree tmp_var; - tmp_var = create_tmp_var (TREE_TYPE (device)); + tmp_var = create_tmp_var (TREE_TYPE (*tp)); if (offloaded) e = split_block_after_labels (new_bb); else @@ -7601,10 +7614,17 @@ expand_omp_target (struct omp_region *region) gsi_insert_after (&gsi, stmt, GSI_CONTINUE_LINKING); gsi = gsi_start_bb (then_bb); - stmt = gimple_build_assign (tmp_var, device); + stmt = gimple_build_assign (tmp_var, *tp); gsi_insert_after (&gsi, stmt, GSI_CONTINUE_LINKING); gsi = gsi_start_bb (else_bb); + if (is_gimple_omp_oacc (entry_stmt)) + stmt = gimple_build_assign (tmp_var, + BIT_IOR_EXPR, + *tp, + build_int_cst (integer_type_node, + GOACC_FLAG_HOST_FALLBACK)); + else stmt = gimple_build_assign (tmp_var, build_int_cst (integer_type_node, GOMP_DEVICE_HOST_FALLBACK)); @@ -7617,12 +7637,15 @@ expand_omp_target (struct omp_region *region) make_edge (then_bb, new_bb, EDGE_FALLTHRU); make_edge (else_bb, new_bb, EDGE_FALLTHRU); - device = tmp_var; + *tp = tmp_var; + gsi = gsi_last_nondebug_bb (new_bb); } else { gsi = gsi_last_nondebug_bb (new_bb); + + if (device != NULL_TREE) device = force_gimple_operand_gsi (&gsi, device, true, NULL_TREE, true, GSI_SAME_STMT); } @@ -7648,6 +7671,16 @@ expand_omp_target (struct omp_region *region) bool tagging = false; /* The maximum number used by any start_ix, without varargs. */ auto_vec args; + if (is_gimple_omp_oacc (entry_stmt)) + { + tree goacc_flags_m = fold_build1 (GOACC_FLAGS_MARSHAL_OP, + TREE_TYPE (goacc_flags), goacc_flags); + goacc_flags_m = force_gimple_operand_gsi (&gsi, goacc_flags_m, true, + NULL_TREE, true, + GSI_SAME_STMT); + args.quick_push (goacc_flags_m); + } + else args.quick_push (device); if (offloaded) args.quick_push (build_fold_addr_expr (child_fn)); Grüße Thomas