Hi Thomas,
Thomas Koenig wrote:
Comments, remarks, suggestions?
I assume you regression-tested (you didn't say).
Yes, it was build with a bootstrapping configuration (with a
non-offloading compiler, but it shouldn't matter here) on
x86_64-gnu-linux and I did run "make check-fortran" in the
main build directory, which covers gcc/testsuite/gfortran*/
and libgomp/testsuite/*fortran/.
Otherwise, I wouldn't have found the the issue in the
OpenACC testcase and patched it. :-)
* * *
Otherwise, I regard the common Fortran code as obvious - and
the OpenMP part covered by my (co)maintainership.
Regarding the Fortran part:
- fndecl = build_decl (input_location,
+ fndecl = build_decl (gfc_get_location (&sym->declared_at),
FUNCTION_DECL, name, type);
Does that have any effect on non-OMP code, and if so, what is it?
If so, could you also add a test case checking for it?
Regarding the first question:
Yes, it affects any (external) procedure declaration at gimple 'tree',
i.e. that code affects most code:
Therefore, if you put a check there — like
gfc_warning_now_at (input_location, 0,
"I was called for %qs, declared at %L", sym->name, &sym->declared_at);
— it will trigger very often.
And the effect is, obviously, to set the location to where the
procedure was declared - instead of using the location the
compiler decided to generate the declaration (which is usually
near the place where it was used for or in a procedure call
or proc-pointer assignment or similar).
* * *
Thus, it affects a lot of code/all code - in principle, but in
practical terms, the effect is small:
* (Nearly) all gfortran front-end diagnostic is done directly on
gfc_{symbol,expr,code} using 'locus', i.e. s->declared_at, e->where
and c->loc.
* It only affects the location of the *declaration* of a procedure
(INTERFACE, EXTERNAL, 'USE' for procedures in a module, when there
is no gsym entry for it)
* After parsing and resolution, nearly every diagnostic in the
compiler - and debugging code - relate to the executable code,
and, possibly, to function definitions or the enclosing function.
But that's unaffected by change.
Obviously, there is some effect - and as the two test cases (one old
for OpenACC and one included in the patch) have, a typical pattern is:
error_at / warning_at (loc, " … some message involving function %qD",
fndecl);
inform (DECL_SOURCE_LOCATION(fndecl), "%qD was declared here", fndecl)
[where %qD prints (for Fortran/C) the name of the function (declaration 'D')
in quotes 'q'. - BTW: the %qD can also be used in gfc_error/gfc_warning.]
* * *
To come to your second question:
* Nearly all existing code runs through that line, i.e. it is somehow
tested for.
* As no regression showed up, seemingly only the existing OpenACC and
the new OpenMP code actually checked for this type of diagnostic.
* As OpenACC is not OpenMP and as both are tested for by default,
I don't see the point for trying to find a new testcase.
* * *
If you are looking for a testcase, I suggest playing with
-fanalyzer as that has output of the form:
You did this here, but that one over here was such and,
by the way, it was declared as such over there.
(However, actually looking at it, analyzer/ is probably not
using the declaration, except for file descriptors and
attribute "access", which is not the case for Fortran. But
maybe in the future it will do?)
Just doing some grepping:
* DECL_SOURCE_LOCATION is used 7 times in gcc/fortran/
(when building a decl or folding (+ 2 on the LHS of an assignment),
My feeling is that none of them is affected by this change.
* DECL_SOURCE_LOCATION is used 287 times in gcc/*.cc and 30 times in
gcc/analyzer/.
Actually, I found a new candidate in tree.cc:
"%qD is deprecated"
which is followed by:
inform (DECL_SOURCE_LOCATION (node), "declared here"); As gfortran uses
EXT_ATTR_DEPRECATED that might be a candidate, but the at least the
existing test cases did not trigger.
Thanks,
Tobias
