On Wed, Nov 14, 2018 at 11:06:04AM +0100, Martin Liška wrote:
> Question I have is about default search locations for the header file. On my
> machine I can
> see:
> access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h",
> R_OK) = -1 ENOENT (No such file or directory)
> access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../x86_64-pc-linux-gnu/lib/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h",
> R_OK) = -1 ENOENT (No such file or directory)
> access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../x86_64-pc-linux-gnu/lib/../lib64/math-vector-fortran.h",
> R_OK) = -1 ENOENT (No such file or directory)
> access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h",
> R_OK) = -1 ENOENT (No such file or directory)
> access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../lib64/math-vector-fortran.h",
> R_OK) = -1 ENOENT (No such file or directory)
> access("/lib/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1
> ENOENT (No such file or directory)
> access("/lib/../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file
> or directory)
> access("/usr/lib/x86_64-pc-linux-gnu/9.0.0/math-vector-fortran.h", R_OK) = -1
> ENOENT (No such file or directory)
> access("/usr/lib/../lib64/math-vector-fortran.h", R_OK) = -1 ENOENT (No such
> file or directory)
> access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../../x86_64-pc-linux-gnu/lib/math-vector-fortran.h",
> R_OK) = -1 ENOENT (No such file or directory)
> access("/home/marxin/bin/gcc2/lib64/gcc/x86_64-pc-linux-gnu/9.0.0/../../../math-vector-fortran.h",
> R_OK) = -1 ENOENT (No such file or directory)
> access("/lib/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or
> directory)
> access("/usr/lib/math-vector-fortran.h", R_OK) = -1 ENOENT (No such file or
> directory)
>
> Aren't these locations desired for libraries, instead of include locations?
That isn't correct indeed.
What about find_a_file (&include_prefixes, ... )?
Though, in the design where to put the file we really need to have multilib
(and multiarch on Debian/Ubuntu) in mind, because e.g. on x86_64-linux you
want to find a m64 version of the header for -m64, but a different for -m32
and there is always the possibility somebody installs a 32-bit gfortran on
x86_64.
> --- a/gcc/config/gnu-user.h
> +++ b/gcc/config/gnu-user.h
> @@ -170,3 +170,6 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
> If not, see
> LD_STATIC_OPTION " --whole-archive -llsan --no-whole-archive " \
> LD_DYNAMIC_OPTION "}}%{!static-liblsan:-llsan}"
> #endif
> +
> +#undef TARGET_F951_NOSTDINC_OPTIONS
> +#define TARGET_F951_NOSTDINC_OPTIONS "%:fortran-header-file(-fpre-include=
> math-vector-fortran.h)"
Too long line, use some \s to split it up.
> + Flags are one of:
> + - omp-simd-notinbranch.
So omp-simd-notinbranch or omp_simd_notinbranch?
Any particular reason for this weird syntax and for not also
supporting inbranch or just simd?
> +
> + When we come here, we have already matched the !GCC$ builtin string. */
> +match
> +gfc_match_gcc_builtin (void)
> +{
> + char builtin[GFC_MAX_SYMBOL_LEN + 1];
> +
> + if (gfc_match_name (builtin) != MATCH_YES)
> + return MATCH_ERROR;
> +
> + gfc_gobble_whitespace ();
> + if (gfc_match ("attributes") != MATCH_YES)
> + return MATCH_ERROR;
> +
> + gfc_gobble_whitespace ();
> + if (gfc_match ("omp_simd_notinbranch") != MATCH_YES)
> + return MATCH_ERROR;
Why so many gfc_match calls? Can't you e.g. just do
if (gfc_match ("%n attributes simd", builtin) != MATCH_YES)
return MATCH_ERROR;
int builtin_kind = 0; /* Or whatever, just want to show the parsing here. */
if (gfc_match ("(notinbranch)") == MATCH_YES)
builtin_kind = -1;
else if (gfc_match ("(inbranch)") == MATCH_YES)
builtin_kind = 1;
The space in gfc_match indicates gfc_gobble_whitespace (), i.e. optionally
eating whitespace (note, in fixed form white space is optionally eaten
pretty much always). If you want a mandatory space, there is "% ".
So it depends in if in fixed form we want to support e.g.
!GCC$ BUI L TIN SINf ATTRI BUT ESSIMD(NOT IN BRANCH)
and in free form e.g.
!gcc$ builtin sinf attributessimd (notinbranch)
I wouldn't use omp_simd because in C/C++ the attribute is called simd.
> +
> + char *r = XNEWVEC (char, strlen (builtin) + 32);
> + sprintf (r, "__builtin_%s", builtin);
> + vectorized_builtins.safe_push (r);
Perhaps make it vector of const char *, int pairs, so that you can
encode no argument (aka inbranch + notinbranch) vs. inbranch vs. notinbranch?
> #define F951_OPTIONS "%(cc1_options) %{J*} \
> - %{!nostdinc:-fintrinsic-modules-path finclude%s}\
> + %{!nostdinc:-fintrinsic-modules-path finclude%s " \
> + TARGET_F951_NOSTDINC_OPTIONS "}\
I wouldn't stick it inside of %{!nostdinc:, but let config/gnu-user.h decide
about that (i.e. wrap it into %{!nostdinc: ... } there).
> --- a/gcc/fortran/lang.opt
> +++ b/gcc/fortran/lang.opt
> @@ -662,6 +662,10 @@ fprotect-parens
> Fortran Var(flag_protect_parens) Init(-1)
> Protect parentheses in expressions.
>
> +fpre-include=
> +Fortran RejectNegative JoinedOrMissing Var(flag_pre_include) Undocumented
> +Path to header file that should be pre-included before each compilation unit.
Why OrMissing? If the argument is missing, that should be an error, you
can't load "".
> --- a/gcc/fortran/scanner.c
> +++ b/gcc/fortran/scanner.c
> @@ -2365,6 +2365,16 @@ load_file (const char *realfilename, const char
> *displayedname, bool initial)
> }
> }
>
> + /* Make a guard to prevent recursive inclusion. */
> + static bool preinclude_done = false;
> + if (!preinclude_done)
> + {
> + preinclude_done = true;
> + if (flag_pre_include != NULL && !load_file (flag_pre_include, NULL,
> + false))
> + exit (FATAL_EXIT_CODE);
> + }
Why in load_file? I'd handle this in gfc_new_file, where it is called just
once. Formatting - would be nicer to put && on the next line and not wrap
load_file args.
> +static const char *
> +find_fortran_header_file (int argc, const char **argv)
> +{
> + if (argc != 2)
> + return NULL;
> +
> + const char *path = find_file (argv[1]);
> + if (path != argv[1])
> + return concat (argv[0], path, NULL);
I wouldn't call it fortran_header_file but fortran_preinclude_file
(both in what appears in spec and the name of the callback).
Plus, the patch lacks testcases. I'd use dg-options "-nostdinc"
so that in that testcase it isn't preincluded, and have one free form and
one fixed form testcase that has various forms of the !gcc$ builtin
for a couple of builtins + some calls to them and check how they are handled
(perhaps limited to the one or two ABIs that support those)?
Jakub
