Hi Julian! On Thu, 16 Jul 2015 16:32:12 +0100, Julian Brown <jul...@codesourcery.com> wrote: > This patch removes the device-specific filtering (for NVidia PTX) from > the parsing stages of the host compiler (for the device_type clause -- > separately for C, C++ and Fortran) in favour of fully parsing the > device_type clauses, but not actually implementing anything for them > (device_type support is a feature that we're not planning to implement > just yet: the existing "support" is something of a red herring). > > With this patch, the parsed device_type clauses will be ready at OMP > lowering time whenever we choose to do something with them (e.g. > transforming them into a representation that can be streamed out and > re-read by the appropriate offload compiler). The representation is > more-or-less the same for all supported languages
Thanks!
> modulo clause ordering.
Is that something that a) doesn't need to be/already has been addressed
(with your patch), or b) still needs to be addressed?
> I've altered the dtype-*.* tests to account for the new behaviour (and
> to not use e.g. mixed-case "nVidia" or "acc_device_nvidia" names, which
> are contrary to the recommendations in the spec).
OpenACC 2.0a indeed seems to suggest that device_type arguments are
case-sensitive -- contrary to the ACC_DEVICE_TYPE environment variable,
which probably is where the idea came from to parse them
case-insensitive.
As to the latter "invalid" names, I thought the idea has been to verify
that the clauses following such device_types clauses are indeed ignored
in the later processing. (Obviously, there should've been comments
indicating that, as otherwise that's very confusing -- as we've just seen
-- due to the similarity to the runtime library's acc_device_* device
type values.)
> OK to apply, or any comments?
Your commit r225927 appears to have caused:
[-PASS:-]{+FAIL: libgomp.fortran/declare-simd-2.f90 -O0 (internal
compiler error)+}
{+FAIL:+} libgomp.fortran/declare-simd-2.f90 -O0 (test for excess errors)
[-PASS:-]{+UNRESOLVED:+} libgomp.fortran/declare-simd-2.f90 -O0
[-execution test-]
[-PASS:-]{+compilation failed to produce executable+}
[same for other optimization levels]
[...]/source-gcc/libgomp/testsuite/libgomp.fortran/declare-simd-3.f90:17:0:
internal compiler error: Segmentation fault
0xc39b6f crash_signal
[...]/source-gcc/gcc/toplev.c:352
0x7043a8 gfc_trans_omp_clauses
[...]/source-gcc/gcc/fortran/trans-openmp.c:2671
0x7049a8 gfc_trans_omp_declare_simd(gfc_namespace*)
[...]/source-gcc/gcc/fortran/trans-openmp.c:4589
0x6b8542 gfc_get_extern_function_decl(gfc_symbol*)
[...]/source-gcc/gcc/fortran/trans-decl.c:2025
0x6b878d gfc_get_extern_function_decl(gfc_symbol*)
[...]/source-gcc/gcc/fortran/trans-decl.c:1820
0x6ce952 conv_function_val
[...]/source-gcc/gcc/fortran/trans-expr.c:3601
0x6ce952 gfc_conv_procedure_call(gfc_se*, gfc_symbol*, gfc_actual_arglist*,
gfc_expr*, vec<tree_node*, va_gc, vl_embed>*)
[...]/source-gcc/gcc/fortran/trans-expr.c:5873
0x6cf4c2 gfc_conv_expr(gfc_se*, gfc_expr*)
[...]/source-gcc/gcc/fortran/trans-expr.c:7391
0x6d71d0 gfc_trans_assignment_1
[...]/source-gcc/gcc/fortran/trans-expr.c:9127
0x692465 trans_code
[...]/source-gcc/gcc/fortran/trans.c:1674
0x6fa457 gfc_trans_omp_code
[...]/source-gcc/gcc/fortran/trans-openmp.c:2711
0x705410 gfc_trans_omp_do
[...]/source-gcc/gcc/fortran/trans-openmp.c:3459
0x707f9f gfc_trans_omp_directive(gfc_code*)
[...]/source-gcc/gcc/fortran/trans-openmp.c:4521
0x6922b7 trans_code
[...]/source-gcc/gcc/fortran/trans.c:1924
0x6c0660 gfc_generate_function_code(gfc_namespace*)
[...]/source-gcc/gcc/fortran/trans-decl.c:6231
0x64d630 translate_all_program_units
[...]/source-gcc/gcc/fortran/parse.c:5523
0x64d630 gfc_parse_file()
[...]/source-gcc/gcc/fortran/parse.c:5728
0x68ef12 gfc_be_parse_file
[...]/source-gcc/gcc/fortran/f95-lang.c:214
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -12439,10 +12439,7 @@ c_parser_oacc_all_clauses (c_parser *parser,
> omp_clause_mask mask,
> c_parser_skip_to_pragma_eol (parser);
>
> if (finish_p)
> - {
> - clauses = c_oacc_filter_device_types (clauses);
> - return c_finish_omp_clauses (clauses, true);
> - }
> + return c_finish_omp_clauses (clauses, true);
>
> return clauses;
> }
In combination with the equivant change to
gcc/cp/parser.c:cp_parser_oacc_all_clauses,
gcc/c-family/c-omp.c:c_oacc_filter_device_types, and transitively also
the struct identifier_hasher and c_oacc_extract_device_id function
preceding it, are now unused. (Not an exhaustive list; have not checked
which other auxilliary functions etc. Cesar has added in his device_type
changes.) Does it make any sense to keep these for later, or dump them
now?
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -12568,6 +12568,10 @@ c_finish_omp_clauses (tree clauses, bool oacc)
> pc = &OMP_CLAUSE_CHAIN (c);
> continue;
>
> + case OMP_CLAUSE_DEVICE_TYPE:
> + pc = &OMP_CLAUSE_DEVICE_TYPE_CLAUSES (c);
> + continue;
> +
> case OMP_CLAUSE_INBRANCH:
> case OMP_CLAUSE_NOTINBRANCH:
> if (branch_seen)
From a quick glance only, this seems to be different from the C++ front
end (have not checked Fortran).
I have not looked at what the front end parsing is now actually doing; is
it just attaching any clauses following a device_type clause to the
latter? (The same should be done for all front ends, obviously. Even if
it's not important right now, because of the sorry diagnostic that will
be emitted later on as soon as there is one device_type clause, this
should best be addressed now, while you still remember what's going on
here ;-) so that there will be no bad surprises once we actually
implement the handling in OMP lowering/streaming/device compilers.)
Do we need manually need to take care to "finalize" (c_finish_omp_clauses
et al.) such "masked" clause chains, or will the right thing happen
automatically?
For reference, C++ does not appear to use OMP_CLAUSE_DEVICE_TYPE_CLAUSES
here:
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -13666,6 +13666,7 @@ tsubst_omp_clauses (tree clauses, bool declare_simd,
> case OMP_CLAUSE_AUTO:
> case OMP_CLAUSE_SEQ:
> case OMP_CLAUSE_TILE:
> + case OMP_CLAUSE_DEVICE_TYPE:
> break;
> default:
> gcc_unreachable ();
> --- a/gcc/cp/semantics.c
> +++ b/gcc/cp/semantics.c
> @@ -5951,6 +5951,7 @@ finish_omp_clauses (tree clauses, bool oacc)
> case OMP_CLAUSE_BIND:
> case OMP_CLAUSE_NOHOST:
> case OMP_CLAUSE_TILE:
> + case OMP_CLAUSE_DEVICE_TYPE:
> break;
>
> case OMP_CLAUSE_INBRANCH:
(I have not checked Fortran.)
I also remember that I had a comment regarding device_type handling in
gcc/c-family/c-omp.c:c_oacc_split_loop_clauses,
<http://news.gmane.org/find-root.php?message_id=%3C87zj2z2e4x.fsf%40schwinge.name%3E>.
Given that these clauses are now no longer being handled to completion in
the front ends, does this need to be addressed?
> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -2028,6 +2028,7 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
> case OMP_CLAUSE_AUTO:
> case OMP_CLAUSE_SEQ:
> case OMP_CLAUSE_TILE:
> + case OMP_CLAUSE_DEVICE_TYPE:
> break;
>
> case OMP_CLAUSE_ALIGNED:
> @@ -2163,6 +2164,7 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
> case OMP_CLAUSE_AUTO:
> case OMP_CLAUSE_SEQ:
> case OMP_CLAUSE_TILE:
> + case OMP_CLAUSE_DEVICE_TYPE:
> break;
>
> case OMP_CLAUSE_DEVICE_RESIDENT:
> @@ -9774,6 +9776,10 @@ expand_omp_target (struct omp_region *region)
> tree t_async;
> int t_wait_idx;
>
> + c = find_omp_clause (clauses, OMP_CLAUSE_DEVICE_TYPE);
> + if (c)
> + sorry ("device_type clause is not supported yet");
> +
> /* Default values for t_async. */
> t_async = fold_convert_loc (gimple_location (entry_stmt),
> integer_type_node,
Typically, sorry messages are emitted in the generic clause handling code
in scan_sharing_clauses.
Grüße,
Thomas
signature.asc
Description: PGP signature
