Re: [PATCH] PR fortran/105310 - ICE when UNION is after the 8th field in a DEC STRUCTURE with -finit-derived -finit-local-zero
On Wed, Apr 20, 2022, 16:27 Harald Anlauf wrote: > > Hi Fritz, > > Am 20.04.22 um 20:03 schrieb Fritz Reese via Fortran: > > See the bug report at gcc dot gnu dot org/bugzilla/show_bug.cgi?id=105310 . > > OK if you add a/the testcase. .. > > As this affects all branches, you may backport the patch as far as > you feel reasonable. (No, I do not use DEC extensions personally.) > > Thanks for the patch! > > Harald Thanks for taking a look Harald. I've committed the test case along with the patch and backported to 9, 10, 11. I would love to backport to 8 as well but I think that branch is closed by now. Cheers, Fritz
[PATCH] PR fortran/105310 - ICE when UNION is after the 8th field in a DEC STRUCTURE with -finit-derived -finit-local-zero
See the bug report at gcc dot gnu dot org/bugzilla/show_bug.cgi?id=105310 .
This code was originally authored by me and the fix is trivial, so I
intend to commit the attached patch in the next few days if there is
no dissent.
The bug is caused by gfc_conv_union_initializer in
gcc/fortran/trans-expr.cc, which accepts a pointer to a vector of
constructor trees (vec*) as an argument, then
appends one or two field constructors to the vector. The problem is
the use of CONSTRUCTOR_APPEND_ELT(v, ...) within
gfc_conv_union_initializer, which modifies the vector pointer v when a
reallocation of the vector occurs, but the pointer is passed by value.
Therefore, when a vector reallocation occurs, the caller's
(gfc_conv_structure) vector pointer is not updated and subsequently
points to freed memory. Chaos ensues.
The bug only occurs when gfc_conv_union_initializer itself triggers
the reallocation, which is whenever the vector is "full"
(v->m_vecpfx.m_alloc == v->m_vecpfx.m_num). Since the vector defaults
to allocating 8 elements and doubles in size for every reallocation,
the bug only occurs when there are 8, 16, 32, etc... fields with
initializers prior to the union, causing the vector of constructors to
be resized when entering gfc_conv_union_initializer. The
-finit-derived and -finit-local-zero options together ensure each
field has an initializer, triggering the bug.
The patch fixes the bug by passing the vector pointer to
gfc_conv_union_initializer by reference, matching the signature of
vec_safe_push from within the CONSTRUCTOR_APPEND_ELT macro.
--
Fritz Reese
diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc
index 06713f24f95..8677a3b0b20 100644
--- a/gcc/fortran/trans-expr.cc
+++ b/gcc/fortran/trans-expr.cc
@@ -9195,7 +9195,7 @@ gfc_trans_structure_assign (tree dest, gfc_expr * expr, bool init, bool coarray)
}
void
-gfc_conv_union_initializer (vec *v,
+gfc_conv_union_initializer (vec *,
gfc_component *un, gfc_expr *init)
{
gfc_constructor *ctor;
[PATCH, libgfortran] Protect the trigd functions in libgfortran from unavailable math functions [PR94586, PR94694]
This patch here is a follow-on to Jakub's in his message "[PATCH]
libgfortran: Provide some further math library fallbacks [PR94694]" at
https://gcc.gnu.org/pipermail/fortran/2020-April/054252.html.
I think this should be committed along with Jakub's patch in response
to PRs 94586 and 94694. Though this patch alone does not fix either
PR, it prevents the build from failing when the degree-valued
trigonometric functions are ultimately unavailable on the target due
to lack of math library builtins.
If HAVE_* is not defined for the math functions required by the trigd
implementations, the intrinsic functions are built only to throw a
runtime_error. Prior to this, the libgfortran build would fail
completely. These intrinsics are extensions, and before the original
trigd patch the library could build even when these math functions
were missing. Therefore, I think the compiler should not fail to build
simply because the intrinsics are unsupported on the target.
In theory, this patch would also fix the trigd build for targets which
don't have a floating-point INFINITY defined, though I am unsure if
any such targets exist.
Furthermore, this patch replaces hard-coded floating point literal
suffixes with the use of GFC_REAL__LITERAL_SUFFIX, as defined by
libgfortran's kinds.h. This could help targets with quirky definitions
for real(16) such as HPUX, as in PR 94586. Similarly, the precision
used for the constants COSD_SMALL, COSD30, et al. for the REAL(16)
case are derived from the precision of GFC_REAL_16 rather than its
long-double-ness, since for example on HPUX long double is actually
128 bits.
This patch also fixes an issue which could have occurred in the
front-end: the front-end attemped to use a host-sized floating-point
literal to represent cosine of 30 degrees. This is of course
nonsensical. The patch replaces this with the use of mpfr functions to
compute the value as sqrt(3)/2 to the correct precision. I include
this in the patch because it is also a target-specific issue with the
degree-valued trigonometric intrinsics.
Jakub has OK'd the patch and I recently authored the original trigd
code being modified here. After his patch is committed I will commit
this one unless others have comments or concerns.
libgfortran/ChangeLog:
2020-04-22 Fritz Reese
* intrinsics/trigd.c, intrinsics/trigd_lib.inc, intrinsics/trigd.inc:
Guard against unavailable math functions.
Use suffixes from kinds.h based on the REAL kind.
gcc/fortran/ChangeLog:
2020-04-22 Fritz Reese
* trigd_fe.inc: Use mpfr to compute cosd(30) rather than a host-
precision floating point literal based on an invalid macro.
---
Fritz Reese
[Note: the only differences between this patch and the one in the PR
is whitespace.]
commit dd9c65c7338ceed5ef30e768e4530d73141918c5
Author: Fritz Reese
Date: Wed Apr 22 11:45:22 2020 -0400
Follow-on to: Provide some further math library fallbacks [PR94694]
Protect the trigd functions in libgfortran from unavailable math functions.
libgfortran/ChangeLog:
2020-04-22 Fritz Reese
* intrinsics/trigd.c, intrinsics/trigd_lib.inc, intrinsics/trigd.inc:
Guard against unavailable math functions.
Use suffixes from kinds.h based on the REAL kind.
gcc/fortran/ChangeLog:
2020-04-22 Fritz Reese
* trigd_fe.inc: Use mpfr to compute cosd(30) rather than a host-
precision floating point literal based on an invalid macro.
diff --git a/gcc/fortran/trigd_fe.inc b/gcc/fortran/trigd_fe.inc
index 78ca4416a21..f94c36773c1 100644
--- a/gcc/fortran/trigd_fe.inc
+++ b/gcc/fortran/trigd_fe.inc
@@ -29,17 +29,20 @@ along with GCC; see the file COPYING3. If not see
#define ISFINITE(x) mpfr_number_p(x)
#define D2R(x) deg2rad(x)
+#define ENABLE_SIND
+#define ENABLE_COSD
+#define ENABLE_TAND
+
#define SIND simplify_sind
#define COSD simplify_cosd
#define TAND simplify_tand
-#ifdef HAVE_GFC_REAL_16
-#define COSD30 8.66025403784438646763723170752936183e-01Q
-#else
-#define COSD30 8.66025403784438646763723170752936183e-01L
-#endif
-
-#define SET_COSD30(x) mpfr_set_ld((x), COSD30, GFC_RND_MODE)
+/* cosd(30) === sqrt(3) / 2. */
+#define SET_COSD30(x) do { \
+mpfr_set_ui (x, 3, GFC_RND_MODE); \
+mpfr_sqrt (x, x, GFC_RND_MODE); \
+mpfr_div_ui (x, x, 2, GFC_RND_MODE); \
+ } while (0)
static RETTYPE SIND (FTYPE);
static RETTYPE COSD (FTYPE);
diff --git a/libgfortran/intrinsics/trigd.c b/libgfortran/intrinsics/trigd.c
index 81699069545..e1c51c7b2ef 100644
--- a/libgfortran/intrinsics/trigd.c
+++ b/libgfortran/intrinsics/trigd.c
@@ -27,6 +27,14 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
#include
+/* Body of library functions which are cannot be implemented on the current
+ * platform because it lacks a capability, such as an underlying trigonometric
+ * function (sin, cos, tan) or C99 floating-point function (fabs, fmod). */
Re: [patch, committed] Add numerous symbol attributes to -fdump-fortran-original
On Mon, Apr 20, 2020 at 1:25 PM Thomas Koenig via Fortran wrote: > > Hello world, > > after finding myself debug a PR where showing all the attributes > of a symbol would have helped enormously (which I realized only > afterwards), I went ahead and added most of the flags to show_attr, > in the hope that this will help all of us debugging some murky corners > of the compiler where attributes are not what they should be. > > If you see anything that is still missing, please freel free > to add. > > Committed as obvious (no user impact) after testing that > the compiler still compiles :-) > > Regards > > Thomas > > 2020-04-20 Thomas Koenig > > * dump-parse-tree.c (show_attr): Add numerous flags, some cleanup. Thanks!! I've always thought these should be handled. --- Fritz Reese
Re: [patch, fortran] Fix PR PR93500
On Fri, Apr 17, 2020 at 10:33 AM Thomas Koenig wrote: > > Hi Fritz, > > > First, it appears if simplify_bound_dim returns _bad_expr (and a > > div/0 occurs) then this code will free _bad_expr. I'm not sure > > whether or not that can actually occur, but it is certainly incorrect, > > since _bad_expr points to static storage. The only other possible > > case is bounds[d] == NULL, in which case the free is a no-op. I > > suggest removing the free call. > > OK, removed. > > > That being said, it looks like the same error condition can occur with > > the lcobound intrinsic. > > I have adjusted the test case so it also checks for the coarray case; > this is handled correctly with a "Divion by zero" error without > changing the patch in that respect. > > So, anything else? If not, I will commit this within a few days. > Let the record show I am unsettled that the error path is different between simplify_bound and simplify_cobound, and that different errors occur for the program and subroutine case. There is probably a root cause somewhere else that should be fixed one day. However! That is not a problem for this PR, nor for stage 4, and is certainly no fault of this patch. Therefore the patch looks OK to me for now. Thank you for your work! --- Fritz Reese
Re: [patch, fortran] Fix PR PR93500
On Thu, Apr 16, 2020 at 7:53 AM Thomas Koenig via Fortran
wrote:
>
> Hello world,
>
> this patch fixes PR PR93500. One part of it is due to
> what Steve wrote in the patch (returning from resolutions when both
> operands are NULL), but that still left a nonsensical error.
> Returning _bad_expr when simplifying bounds resulted in the
> division by zero error actually reaching the user.
>
> As to why there is an extra error when this is done in the main
> program, as compared to a subroutine, I don't know, but I do not
> particularly care. What is important is that the first error message
> is clear and reaches the user.
>
> Regression-tested. OK for trunk?
How odd. It seems something in the procedure matching routine fails to
free the symbol node for "a", while this _is_ done for the program
case. A bug for another day...
IMO a more clear test case uses "implicit none", which reveals the
more sensible "Symbol .a. at ... has no IMPLICIT type" (at least for
the program case, where a second error is displayed). One can see
another variation of this with a declaration like "integer,
dimension(lbound(a)) :: c" which gives a similar error "Symbol .a. is
used before it is typed at ...".
Regarding the new code in simplify.c:
bounds[d] = simplify_bound_dim([...])
if (bounds[d] == NULL || bounds[d] == _bad_expr)
{
[...]
if (gfc_seen_div0)
{
gfc_free_expr (bounds[d]);
return _bad_expr;
}
[...]
First, it appears if simplify_bound_dim returns _bad_expr (and a
div/0 occurs) then this code will free _bad_expr. I'm not sure
whether or not that can actually occur, but it is certainly incorrect,
since _bad_expr points to static storage. The only other possible
case is bounds[d] == NULL, in which case the free is a no-op. I
suggest removing the free call.
That being said, it looks like the same error condition can occur with
the lcobound intrinsic. I see code inside simplify_cobound nearly
identical to that in simplify_bound which is not guarded by the new
gfc_seen_div0 check. Someone more familiar with coarrays may be able
to generate a testcase which exhibits the same regression using
lcobound, but I am confident it can occur. This suggests to me that
the check is better placed in simplify_bound_dim, which both
simplify_bound and simplify_cobound call. If simplify_bound_dim
returns _bad_expr, it appears both routines should continue to do
the right thing already (which would not include freeing
gfc_bad_expr). It is the call to gfc_resolve_array_spec within
simplify_bound_dim which signals the div0, so I believe the div0 check
should be inserted right here (around line 4075). How about the
following patch to simplify.c instead (which appears to have the
fortunate side-effect of fixing a formerly leaked result expression):
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index d5703e38251..5395694dc67 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4073,7 +4073,13 @@ simplify_bound_dim (gfc_expr *array, gfc_expr
*kind, int d, int upper,
gcc_assert (as);
if (!gfc_resolve_array_spec (as, 0))
-return NULL;
+{
+ gfc_free_expr (result);
+ if (gfc_seen_div0)
+ return _bad_expr;
+ else
+ return NULL;
+}
/* The last dimension of an assumed-size array is special. */
if ((!coarray && d == as->rank && as->type == AS_ASSUMED_SIZE && !upper)
---
Fritz Reese
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index d5703e38251..5395694dc67 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4073,7 +4073,13 @@ simplify_bound_dim (gfc_expr *array, gfc_expr *kind, int
d, int upper,
gcc_assert (as);
if (!gfc_resolve_array_spec (as, 0))
-return NULL;
+{
+ gfc_free_expr (result);
+ if (gfc_seen_div0)
+ return _bad_expr;
+ else
+ return NULL;
+}
/* The last dimension of an assumed-size array is special. */
if ((!coarray && d == as->rank && as->type == AS_ASSUMED_SIZE && !upper)
Re: [patch, fortran] Fix ICE on invalid, PR 94090
On Wed, Apr 15, 2020 at 1:47 PM Thomas Koenig wrote: > > Hi Fritz, > > > While you're touching the code anyway, how would you feel about > > replacing the nearby "goto done"s with a chain of "else if"? There's > > really no reason I can see for goto here, since the block following > > the conditions is already "done". > > I think this would really be pushing things at stage 4. Theoretically, > we are expected to do regression and documentation fixes only. > Since Fortran is not release critical, we are welcome to break > our compiler if we want to :-) but we should really try to > restrict outselves at least a little. The little cleanup that I proposed > is also borderline (but maybe a little less so), so I'd rather commit > as I proposed. > > Any other comments? If not, I'll commit in a couple of days. [...] Fair enough. No further comments, that patch looks good. Thanks! --- Fritz
Re: ICE on wrong code [PR94192] commit
On Tue, Apr 14, 2020 at 7:54 AM Linus König wrote: > > Hi all, > > the PR has just recently been committed. Paul gave his offline approval. > I only realized recently, that the NULL pointer check can actually be > ommitted if the check is moved in the code. In the commit however, it is > still right below the declaration. I believe both ways work fine. It is > still possible to change it. > > Best regards, > > Linus König > I committed the simplified check. Thanks for your work, Linus. --- Fritz
Re: [patch, fortran] Fix ICE on invalid, PR 94090
> Yes. Looking back at the code, I think it can also be cleaned up
> a little - turning the error to warnings is only needed on that
> particular branch, and resetting it to the default can also
> happen there, and at the target of a goto statement.
>
> So, here's an updated patch. OK for trunk?
>
> Regards
>
> Thomas
Looks great, thank you for the cleanup!
While you're touching the code anyway, how would you feel about
replacing the nearby "goto done"s with a chain of "else if"? There's
really no reason I can see for goto here, since the block following
the conditions is already "done". Here (and attached) is a diff on top
of your latest changes, in case you think it's appropriate:
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 2371ab23645..617e8d01a59 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -2511,6 +2511,7 @@ resolve_global_procedure (gfc_symbol *sym, locus
*where, int sub)
gfc_namespace *ns;
enum gfc_symbol_type type;
char reason[200];
+ bool bad_result_characteristics;
type = sub ? GSYM_SUBROUTINE : GSYM_FUNCTION;
@@ -2586,23 +2587,16 @@ resolve_global_procedure (gfc_symbol *sym,
locus *where, int sub)
}
if (sym->attr.function && !gfc_compare_types (>ts, _sym->ts))
-{
- gfc_error ("Return type mismatch of function %qs at %L (%s/%s)",
- sym->name, >declared_at, gfc_typename (>ts),
- gfc_typename (_sym->ts));
- goto done;
-}
+gfc_error ("Return type mismatch of function %qs at %L (%s/%s)",
+ sym->name, >declared_at, gfc_typename (>ts),
+ gfc_typename (_sym->ts));
- if (sym->attr.if_source == IFSRC_UNKNOWN
+ else if (sym->attr.if_source == IFSRC_UNKNOWN
&& gfc_explicit_interface_required (def_sym, reason, sizeof(reason)))
-{
- gfc_error ("Explicit interface required for %qs at %L: %s",
- sym->name, >declared_at, reason);
- goto done;
-}
+gfc_error ("Explicit interface required for %qs at %L: %s",
+ sym->name, >declared_at, reason);
- bool bad_result_characteristics;
- if (!gfc_compare_interfaces (sym, def_sym, sym->name, 0, 1,
+ else if (!gfc_compare_interfaces (sym, def_sym, sym->name, 0, 1,
reason, sizeof(reason), NULL, NULL,
_result_characteristics))
{
@@ -2617,12 +2611,9 @@ resolve_global_procedure (gfc_symbol *sym,
locus *where, int sub)
gfc_error ("Interface mismatch in global procedure %qs at %L: %s",
sym->name, >declared_at, reason);
gfc_errors_to_warnings (false);
- goto done;
}
}
-done:
-
if (gsym->type == GSYM_UNKNOWN)
{
gsym->type = type;
---
Even if you don't want to include this, your patch LGTM. Thanks again.
---
Fritz Reese
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 2371ab23645..617e8d01a59 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -2511,6 +2511,7 @@ resolve_global_procedure (gfc_symbol *sym, locus *where,
int sub)
gfc_namespace *ns;
enum gfc_symbol_type type;
char reason[200];
+ bool bad_result_characteristics;
type = sub ? GSYM_SUBROUTINE : GSYM_FUNCTION;
@@ -2586,23 +2587,16 @@ resolve_global_procedure (gfc_symbol *sym, locus
*where, int sub)
}
if (sym->attr.function && !gfc_compare_types (>ts, _sym->ts))
- {
- gfc_error ("Return type mismatch of function %qs at %L (%s/%s)",
-sym->name, >declared_at, gfc_typename (>ts),
-gfc_typename (_sym->ts));
- goto done;
- }
+ gfc_error ("Return type mismatch of function %qs at %L (%s/%s)",
+ sym->name, >declared_at, gfc_typename (>ts),
+ gfc_typename (_sym->ts));
- if (sym->attr.if_source == IFSRC_UNKNOWN
+ else if (sym->attr.if_source == IFSRC_UNKNOWN
&& gfc_explicit_interface_required (def_sym, reason, sizeof(reason)))
- {
- gfc_error ("Explicit interface required for %qs at %L: %s",
-sym->name, >declared_at, reason);
- goto done;
- }
+ gfc_error ("Explicit interface required for %qs at %L: %s",
+ sym->name, >declared_at, reason);
- bool bad_result_characteristics;
- if (!gfc_compare_interfaces (sym, def_sym, sym->name, 0, 1,
+ else if (!gfc_compare_interfaces (sym, def_sym, sym->name, 0, 1,
reason, sizeof(reason), NULL, NULL,
_result_characteristics))
{
@@ -2617,12 +2611,9 @@ resolve_global_procedure (gfc_symbol *sym, locus *where,
int sub)
gfc_error ("Interface mismatch in global procedure %qs at %L: %s",
sym->name, >declared_at, reason);
gfc_errors_to_warnings (false);
- goto done;
}
}
-done:
-
if (gsym->type == GSYM_UNKNOWN)
{
gsym->type = type;
Re: [patch, fortran] Fix ICE on invalid, PR 94090
On Mon, Apr 13, 2020 at 10:20 AM Thomas Koenig via Fortran
wrote:
>
> Hello world,
>
> the attached patch fixes an ICE on invalid: When the return type of
> a function was misdeclared with a wrong rank, we issued a warning,
> but not an error (unless with -pedantic); later on, an ICE ensued.
>
> Nothing good can come from wrongly declaring a function type
> (considering the ABI), so I changed that into a hard error.
>
> OK for trunk?
>
> Regards
>
> Thomas
>
> 2020-04-13 Thomas Koenig
>
> PR fortran/94090
> * gfortran.dg (gfc_compare_interfaces): Add
> optional argument bad_result_characteristics.
> * interface.c (gfc_check_result_characteristics): Fix
> whitespace.
> (gfc_compare_interfaces): Handle new argument; return
> true if function return values are wrong.
> * resolve.c (resolve_global_procedure): Hard error if
> the return value of a function is wrong.
>
> 2020-04-13 Thomas Koenig
>
> PR fortran/94090
> * gfortran.dg/interface_46.f90: New test.
Thomas,
I agree with your assessment and the spirit of the patch.
I wonder: could you simply replace the gfc_error_opt(0, ...) call with
gfc_error? From what I can tell, gfc_error() is simply a short-cut for
gfc_error_opt(0, ...). This has the nice side-effects of reducing the
annoying 81-character line, and using only one copy of the error call:
@@ -2605,11 +2605,19 @@ resolve_global_procedure (gfc_symbol *sym,
locus *where, int sub)
/* Turn erros into warnings with -std=gnu and -std=legacy. */
gfc_errors_to_warnings (true);
+ /* If a function returns a wrong type, this can lead to
+all kinds of ICEs and wrong code; issue a hard error
+in this case. */
+
+ bool bad_result_characteristics;
if (!gfc_compare_interfaces (sym, def_sym, sym->name, 0, 1,
- reason, sizeof(reason), NULL, NULL))
+ reason, sizeof(reason), NULL, NULL,
+ _result_characteristics))
{
- gfc_error_opt (0, "Interface mismatch in global procedure %qs at %L:"
-" %s", sym->name, >declared_at, reason);
+ if (bad_result_characteristics)
+ gfc_errors_to_warnings (false);
+ gfc_error ("Interface mismatch in global procedure %qs at %L:"
+" %s", sym->name, >declared_at, reason);
goto done;
}
}
Otherwise LGTM, thanks for the fix.
---
Fritz Reese
Re: ICE on wrong code [PR94192]
On Sat, Apr 11, 2020 at 10:27 AM Linus König wrote: > > Hi, > > Here is the patch with some of the null pointer tests removed. > > This is regression-tested. ChangeLog and test case are as in > https://gcc.gnu.org/pipermail/fortran/2020-April/054193.html . Thanks. Sorry I missed the ChangeLog entry and tests in my previous email. Those look good. > The list of test cases that fail without the remaining NULL > check is below. Is this OK for trunk? Thank you for the test listing. I apologize for being pedantic, but the remaining NULL check is still superfluous. The check can be removed simply by moving the new code past the BT_CLASS and EXPR_VARIABLE checks, just before the loop, like in my previous diffs. The idea is that when array->expr_type is EXPR_VARIABLE, array->symtree is guaranteed not to be NULL. If this invariant was violated, there are at least five functions in simplify.c alone which would segfault, including simplify_bound, even with this patch. Therefore I prefer not to check array->symtree for NULL before the EXPR_VARIABLE test. With that change I will OK the patch. If you and the regression tests concur, I can commit for you. I believe this is appropriate for backporting to 8 and 9 as well. Thanks again for your work. --- Fritz
Re: ICE on wrong colde [PR94192]
On Fri, Apr 10, 2020 at 8:27 AM Linus König wrote:
>
> Hi,
>
> I fixed the style issues. However, omitting the checks for NULL produced
> several regressions in my previous tests.
>
The style looks good. Please share testcases which exhibit the
regressions. They will also need to be included in
gcc/testsuite/gfortran.dg/ as part of the final commit. I am not aware
of a good source of documentation for writing the testcases
appropriate for DejaGnu, but you can examine other testcases to see
some examples. I can help you format the testcases as well. For the
final commit, a ChangeLog entry will also be necessary: take a look at
gcc/fortran/ChangeLog and match the format of previous entries. In
this case an entry for this patch might look something like this:
2020-04-10 Linus König
PR fortran/94192
* resolve.c (resolve_fl_var_and_proc): Mark invalid array pointer
symbol with error.
* simplify.c (simplify_bound): Don't simplify if an error was
already reported.
With this patch, I am not convinced that NULL components would be
handled properly. It appears that if array is NULL, the new check will
not pass and the next line "array->ts.type == BT_CLASS" will segfault.
If array->symtree is NULL and the next two checks for BT_CLASS and
EXPR_VARIABLE do not pass, then the line "as = array->symtree->n.sym"
will segfault. Perhaps seeing the relevant testcases will clarify the
conditions which this patch covers.
If you believe the NULL guards are required, please consider the
following replacement for the code in simplify_bound, which would
protect the surrounding code as well:
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index f63f63c9ef6..20d02210971 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4155,10 +4155,14 @@ returnNull:
static gfc_expr *
simplify_bound (gfc_expr *array, gfc_expr *dim, gfc_expr *kind, int upper)
{
+ gfc_symbol *array_sym;
gfc_ref *ref;
gfc_array_spec *as;
int d;
+ if (!array)
+return NULL;
+
if (array->ts.type == BT_CLASS)
return NULL;
@@ -4169,8 +4173,16 @@ simplify_bound (gfc_expr *array, gfc_expr *dim,
gfc_expr *kind, int upper)
goto done;
}
+ if (!array->symtree)
+return NULL;
+
+ /* Do not attempt to resolve if error has already been issued. */
+ array_sym = array->symtree->n.sym;
+ if (!array_sym || array_sym->error)
+return NULL;
+
/* Follow any component references. */
- as = array->symtree->n.sym->as;
+ as = array_sym->as;
for (ref = array->ref; ref; ref = ref->next)
{
switch (ref->type)
---
Fritz Reese
Re: [PATCH, Fortran] -- PR fortran/87923 -- fix ICE when resolving I/O tags and simplify io.c
On Fri, Apr 10, 2020 at 8:14 AM Rainer Orth wrote: > > Hi Fritz, [...] > one new testcases comes up as UNRESOLVED everywhere: > > +UNRESOLVED: gfortran.dg/asynchronous_5.f03 -O scan-tree-dump-not > original "volatile.*?ivar_noasync" > +UNRESOLVED: gfortran.dg/asynchronous_5.f03 -O scan-tree-dump-times > original "volatile.*?ccvar_async" 1 > +UNRESOLVED: gfortran.dg/asynchronous_5.f03 -O scan-tree-dump-times > original "volatile.*?darrvar_async" 1 > +UNRESOLVED: gfortran.dg/asynchronous_5.f03 -O scan-tree-dump-times > original "volatile.*?dvar_async" 1 > +UNRESOLVED: gfortran.dg/asynchronous_5.f03 -O scan-tree-dump-times > original "volatile.*?ivar_async" 1 > +UNRESOLVED: gfortran.dg/asynchronous_5.f03 -O scan-tree-dump-times > original "volatile.*?lvar_async" 1 > +UNRESOLVED: gfortran.dg/asynchronous_5.f03 -O scan-tree-dump-times > original "volatile.*?rvar_async" 1 > > gfortran.dg/asynchronous_5.f03 -O : dump file does not exist > > It has several scan-tree-dump* checks, but no corresponding > -fdump-tree-* option. Please fix (and make sure not to look only for > FAILs during regtesting in the future). > > Rainer Ah! My mistake... I will fix and look for this in the future. Fritz
Re: [PATCH, Fortran] -- PR fortran/87923 -- fix ICE when resolving I/O tags and simplify io.c
Tobias, Thanks very much for the review. On Thu, Apr 9, 2020 at 5:21 AM Tobias Burnus wrote: > > Hi, > > On 4/6/20 7:25 PM, Fritz Reese via Fortran wrote: > > > The attached patch fixes PR 87923 while also simplifying the code in > > io.c. > > Thanks for the work, which looks great; it is a bit lengthy > but mostly moving code or mechanical (%C → %L). > It also has an impressive amount of new test cases! I also wished the patch could be easier on the eyes, but alas sometimes this is the price of progress. :-) > * First line in git commit "Fix fortran/87923 -- ICE(s) when resolving I/O > tags." >It helps with doing patch archeology if they are the same – or if the GIT > one >is a substring of the email subject. (If it is about a PR, searching for > the PR >usually works, but also not al emails have the PR number in the subject.) >For this patch, you use: >email: "[PATCH, Fortran] -- PR fortran/87923 -- fix ICE when resolving I/O > tags and simplify io.c" >GIT: "Fix fortran/87923 -- ICE(s) when resolving I/O tags." Yes, that is a good point. I will alter the commit summary to match the email subject. > * Please check whether the ChangeLog lines are too long; I didn't count >but it looks as if they might be too long. For sure, they >were too long for your mail program … I copied the git commit message from the log, which git formats with an extra level of indentation. I wrapped the raw ChangeLog entries and commit message to 80 characters, but after the extra indentation my mail client indeed wrapped the lines during post-processing. I suppose I should wrap these each to 76 to account for git's indentation. That certainly makes "git log" look better. > * In the following comment, you have two empty tailing lines: > > + Tag expressions are already resolved by resolve_tag, which includes > + verifying the type, that they are scalar, and verifying that BT_CHARACTER > + tags are of default kind. > + > + */ Oops! I will commit the patch with these fixes rebased on master after one final build+test. Thanks again for taking a look. Cheers, --- Fritz Reese
Re: [patch,committed] Move gfortran.dg/dec_math_5.f90 to ./ieee/ (was: Re: PATCH -- Fix degree trignometric functions)
Andreas, thank you for the report. Tobias, thank you for the fix. --- Fritz Reese On Wed, Apr 8, 2020 at 12:02 PM Tobias Burnus wrote: > > Hi Andreas, > > thanks for the report. In principle, it would be helpful to know on > which target you are running the test case. > > However, I assume that either of the following went wrong: > * Target does not support IEEE > * It supports it, but gfortran's intrinsic search path does >not point to directory in which the ieee modules are > > That's solved by moving the test case to the ieee/ subdirectory > which has a check whether IEEE works and also sets the intrinsic > module include path (-fintrinsic-modules-path $specpath/libgfortran/). > > I have now committed it as obvious – but I would be good when you > can confirm that it works (PASS if it should support IEEE or, if not, > UNSUPPORTED). > > See r10-7631-gfaa0817311f43e0d4d223d53c816b0c74ec35c4e > or attachment. > > Cheers, > > Tobias > > On 4/8/20 5:04 PM, Andreas Schwab wrote: > > > FAIL: gfortran.dg/dec_math_5.f90 -O0 (test for excess errors) > > Excess errors: > > /opt/gcc/gcc-20200408/gcc/testsuite/gfortran.dg/dec_math_5.f90:132:9: Fatal > > Error: Cannot open module file 'ieee_arithmetic.mod' for reading at (1): No > > such file or directory > > compilation terminated. > > > > Andreas. > > > - > Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany > Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, > Alexander Walter
Re: Fix an ICE found in PR93686
On Mon, Apr 6, 2020 at 5:47 PM Steve Kargl wrote: [...] > BTW, if you haven't committed the degree trig functions, > then I think you should to get the fixes in for 10.1. Roger that. --- Fritz
Re: Fix an ICE found in PR93686
On Sat, Apr 4, 2020 at 2:58 PM Steve Kargl via Fortran wrote: > > This patch fixes the ICE found in PR93686. > > > Index: gcc/fortran/decl.c > === > --- gcc/fortran/decl.c (revision 280157) > +++ gcc/fortran/decl.c (working copy) > @@ -696,6 +696,10 @@ gfc_match_data (void) > /* F2008:C567 (R536) A data-i-do-object or a variable that appears > as a data-stmt-object shall not be an object designator in which > a pointer appears other than as the entire rightmost part-ref. > */ > + if (!e->ref && e->ts.type == BT_DERIVED > + && e->symtree->n.sym->attr.pointer) > + goto partref; > + > ref = e->ref; > if (e->symtree->n.sym->ts.type == BT_DERIVED > && e->symtree->n.sym->attr.pointer > > -- > Steve LGTM, thanks for the patch. I will commit along with the testcases from the PR. --- Fritz
[PATCH, Fortran] -- PR fortran/87923 -- fix ICE when resolving I/O tags and simplify io.c
All, The attached patch fixes PR 87923 while also simplifying the code in io.c. I do say this patch simplifies io.c because it is true. This patch causes more deletions than insertions to the file -- a rare sight: gcc/fortran/io.c | 859 --- 1 file changed, 381 insertions(+), 478 deletions(-) Over the years various special cases have been introduced which are not necessary. The constraints for I/O statements are verified in several different places, and in fact some constraints (like whether an I/O tag is a scalar default-kind character) are checked as many as three times. This patches simplifies the code by moving all checks not necessary for matching out of the matching phase and into the resolution phase. The resolve_tag function already checks several constraints for I/O tags, including type and kind, which were previously checked redundantly in other places. This patch also improves error reporting by providing the correct locus for I/O statement error messages, and by providing a more detailed error message when a tag which requires an init-expr is not a valid init-expr. The patch also increases test coverage of I/O statements, especially for I/O tags, by incorporating testcases provided in PRs from the past which the removed code originally addressed: specifically, PRs 66724 and 66725. With the patch applied on the current master (c72a1b6f8b2...) all regression tests with check-fortran pass, including the new ones. OK for master? commit 5a403f4e8e77123994ca1ed05e8f10877423fe55 Author: Fritz Reese Date: Mon Apr 6 12:13:48 2020 -0400 Fix fortran/87923 -- ICE(s) when resolving I/O tags. 2020-04-06 Fritz Reese This patch also reorganizes I/O checking code. Checks which were done in the matching phase which do not affect the match result are moved to the resolution phase. Checks which were duplicated in both the matching phase and resolution phase have been reduced to one check in the resolution phase. Another section of code which used a global async_io_dt flag to check for and assign the asynchronous attribute to variables used in asynchronous I/O has been simplified. Furthermore, this patch improves error reporting and expands test coverage of I/O tags: - "TAG must be an initialization expression" reported by io.c (check_io_constraints) is replaced with an error from expr.c (gfc_reduce_init_expr) indicating _why_ the expression is not a valid initialization expression. - Several distinct error messages regarding the check for scalar + character + default kind have been unified to one message reported by resolve_tag or check_*_constraints. gcc/fortran/ChangeLog: PR fortran/87923 * gfortran.h (gfc_resolve_open, gfc_resolve_close): Add locus parameter. (gfc_resolve_dt): Add code parameter. * io.c (async_io_dt, check_char_variable, is_char_type): Removed. (resolve_tag_format): Add locus to error message regarding zero-sized array in FORMAT tag. (check_open_constraints, check_close_constraints): New functions called at resolution time. (gfc_match_open, gfc_match_close, match_io): Move checks which don't affect the match result to new functions check_open_constraints, check_close_constraints, check_io_constraints. (gfc_resolve_open, gfc_resolve_close): Call new functions check_open_constraints, check_close_constraints after all tags have been independently resolved. Remove duplicate constraints which are already verified by resolve_tag. Explicitly pass locus to all error reports. (compare_to_allowed_values): Add locus parameter and provide explicit locus all error reports. (match_open_element, match_close_element, match_file_element, match_dt_element, match_inquire_element): Remove redundant special cases for ASYNCHRONOUS and IOMSG tags. (gfc_resolve_dt): Remove redundant special case for format expression. Call check_io_constraints, forwarding an I/O list as the io_code parameter if present. (check_io_constraints): Change return type to bool. Pass explicit locus to error reports. Move generic checks after tag-specific checks, since errors are no longer buffered. Move simplification of format string to match_io. Remove redundant checks which are verified by resolve_tag. Remove usage of async_io_dt flag and explicitly mark symbols used in asynchronous I/O with the asynchronous attribute. * resolve.c (resolve_transfer, resolve_fl_namelist): Remove checks for async_io_dt flag. This is now done in io.c (check_io_constraints). (gfc_resolve_code): Pass
Re: ICE on wrong code [PR94192]
On Mon, Apr 6, 2020 at 8:51 AM Linus König wrote:
>
> Hi all,
>
> I'm new to gcc and this is my first patch. The idea is not have another
> resolution of a pointer if an error has occurred previously. I hope this
> meets all the criteria for a patch. In case anything is missing or
> wrong, I'm open to add to or change the patch.
First, thanks for your work.
In principle the patch looks fine. I was surprised that I could find
no other code that uses gfc_symbol::error; however, your usage of it
seems appropriate.
A note about style, there should be spaces between logical operators:
/* Do not attempt to resolve if error has already been issued. */
- if (array&>symtree&>symtree->n.sym)
+ if (array && array->symtree && array->symtree->n.sym)
{
Additionally, in simplify.c (simplify_bound) you can see the
surrounding code does not check array or array->symtree->n.sym for
NULL. One can assume this precondition holds and factor the code
slightly simpler. I believe also it is sufficient to check only "if
(array_sym->error)". By the time sym->error is set in
resolve_fl_var_and_proc, sym->resolved must already be set to 1
because this is the first step in resolve_symbol.
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 66ed925c10d..84aeafc1e8b 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -4090,6 +4090,7 @@ static gfc_expr *
simplify_bound (gfc_expr *array, gfc_expr *dim, gfc_expr *kind, int upper)
{
gfc_ref *ref;
+ gfc_symbol *array_sym;
gfc_array_spec *as;
int d;
@@ -4103,8 +4104,13 @@ simplify_bound (gfc_expr *array, gfc_expr *dim,
gfc_expr *kind, int upper)
goto done;
}
+ /* Do not attempt to resolve if error has already been issued. */
+ array_sym = array->symtree->n.sym;
+ if (array_sym->error)
+return NULL;
+
/* Follow any component references. */
- as = array->symtree->n.sym->as;
+ as = array_sym->as;
for (ref = array->ref; ref; ref = ref->next)
{
switch (ref->type)
Thanks again for your patch.
---
Fritz Reese
Re: PING -- [PATCH, fortran] PR 85982 -- ICE in resolve_component
Tobias,
Thank you for the information. I didn't think about translations. I'll
post a new version and commit shortly.
Cheers,
Fritz
On Thu, Apr 2, 2020 at 3:50 AM Tobias Burnus wrote:
>
> In principle, I like the patch. However, I think one should
> replace
>
> gfc_error ("Attribute at %L is not allowed in a %s definition",
>…, state_name
>
> by something like:
>
> bool is_type = gfc_current_state () == COMP_DERIVED;
> gfc_error (is_type ? G_("Attribute at %L is not allowed in a TYPE definition")
>: G_("Attribute at %L is not allowed in a STRUCTURE
> definition"),
>…
>
> Reason: (a) This makes translation simpler; e.g. 'structure' and 'type' have
> different gender in several European languages. (Albeit in this case the
> gender of 'definition' dominates in the cases I checked.)
> (b) For TYPE, the string won't change such that the existing translations
> still work – even if the update for STRUCTURE won't make it for the release.
>
> Otherwise it looks good to me, including the test case in your follow-up
> email.
>
> Cheers,
>
> Tobias
>
> On 4/1/20 7:19 PM, Fritz Reese via Fortran wrote:
>
> > This simple patch was submitted some time ago (over 1 year), but got
> > lost without review. I have lately rebased and tested, and the patch
> > is still good. Is this OK to commit to trunk and for backport? I'd
> > like to port as far back as 7.
> >
> > ---
> > Fritz Reese
> >
> > gcc/ChangeLog:
> > 2020-04-01 Fritz Reese
> >
> > PR fortran/85982
> > * fortran/decl.c (match_attr_spec): Lump COMP_STRUCTURE/COMP_MAP
> > into
> > attribute checking used by TYPE.
> >
> > gcc/testsuite/ChangeLog:
> > 2020-04-01 Fritz Reese
> >
> > PR fortran/85982
> > * gfortran.dg/dec_structure_28.f90: New test.
> -
> Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
> Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung,
> Alexander Walter
Re: PING -- [PATCH, fortran] PR 85982 -- ICE in resolve_component
On Wed, Apr 1, 2020 at 1:19 PM Fritz Reese wrote:
[...]
> is still good. Is this OK to commit to trunk and for backport? I'd
> like to port as far back as 7.
I realized 7 branch is closed. I would backport to 8.
> gcc/testsuite/ChangeLog:
> 2020-04-01 Fritz Reese
>
>PR fortran/85982
>* gfortran.dg/dec_structure_28.f90: New test.
Forgot to include the testcase in the patch. Testcase is attached.
! { dg-do compile }
! { dg-options "-fdec-structure -fdec-static" }
!
! PR fortran/85982
!
! Test a regression wherein some component attributes were erroneously accepted
! within a DEC structure.
!
structure /s/
integer :: a
integer, intent(in) :: b ! { dg-error "is not allowed" }
integer, intent(out) :: c ! { dg-error "is not allowed" }
integer, intent(inout) :: d ! { dg-error "is not allowed" }
integer, dimension(1,1) :: e ! OK
integer, external, pointer :: f ! { dg-error "is not allowed" }
integer, intrinsic :: f ! { dg-error "is not allowed" }
integer, optional :: g ! { dg-error "is not allowed" }
integer, parameter :: h ! { dg-error "is not allowed" }
integer, protected :: i ! { dg-error "is not allowed" }
integer, private :: j ! { dg-error "is not allowed" }
integer, static :: k ! { dg-error "is not allowed" }
integer, automatic :: l ! { dg-error "is not allowed" }
integer, public :: m ! { dg-error "is not allowed" }
integer, save :: n ! { dg-error "is not allowed" }
integer, target :: o ! { dg-error "is not allowed" }
integer, value :: p ! { dg-error "is not allowed" }
integer, volatile :: q ! { dg-error "is not allowed" }
integer, bind(c) :: r ! { dg-error "is not allowed" }
integer, asynchronous :: t ! { dg-error "is not allowed" }
character(len=3) :: v ! OK
integer(kind=4) :: w ! OK
end structure
end
PING -- [PATCH, fortran] PR 85982 -- ICE in resolve_component
This simple patch was submitted some time ago (over 1 year), but got
lost without review. I have lately rebased and tested, and the patch
is still good. Is this OK to commit to trunk and for backport? I'd
like to port as far back as 7.
---
Fritz Reese
gcc/ChangeLog:
2020-04-01 Fritz Reese
PR fortran/85982
* fortran/decl.c (match_attr_spec): Lump COMP_STRUCTURE/COMP_MAP into
attribute checking used by TYPE.
gcc/testsuite/ChangeLog:
2020-04-01 Fritz Reese
PR fortran/85982
* gfortran.dg/dec_structure_28.f90: New test.
commit 03ade661deaa606b005304814be9f723158ed55f
Author: Fritz Reese
Date: Fri Mar 20 13:03:42 2020 -0400
Fix for fortran/85982
diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c
index 2f458c4faac..05503b2d3c7 100644
--- a/gcc/fortran/decl.c
+++ b/gcc/fortran/decl.c
@@ -5408,15 +5408,18 @@ match_attr_spec (void)
if (d == DECL_STATIC && seen[DECL_SAVE])
continue;
- if (gfc_current_state () == COMP_DERIVED
+ if (gfc_comp_struct (gfc_current_state ())
&& d != DECL_DIMENSION && d != DECL_CODIMENSION
&& d != DECL_POINTER && d != DECL_PRIVATE
&& d != DECL_PUBLIC && d != DECL_CONTIGUOUS && d != DECL_NONE)
{
+ const char* const state_name = (gfc_current_state () == COMP_DERIVED
+ ? "TYPE" : "STRUCTURE");
if (d == DECL_ALLOCATABLE)
{
if (!gfc_notify_std (GFC_STD_F2003, "ALLOCATABLE "
- "attribute at %C in a TYPE definition"))
+ "attribute at %C in a %s definition",
+ state_name))
{
m = MATCH_ERROR;
goto cleanup;
@@ -5425,7 +5428,8 @@ match_attr_spec (void)
else if (d == DECL_KIND)
{
if (!gfc_notify_std (GFC_STD_F2003, "KIND "
- "attribute at %C in a TYPE definition"))
+ "attribute at %C in a %s definition",
+ state_name))
{
m = MATCH_ERROR;
goto cleanup;
@@ -5449,7 +5453,8 @@ match_attr_spec (void)
else if (d == DECL_LEN)
{
if (!gfc_notify_std (GFC_STD_F2003, "LEN "
- "attribute at %C in a TYPE definition"))
+ "attribute at %C in a %s definition",
+ state_name))
{
m = MATCH_ERROR;
goto cleanup;
@@ -5472,8 +5477,8 @@ match_attr_spec (void)
}
else
{
- gfc_error ("Attribute at %L is not allowed in a TYPE definition",
- _at[d]);
+ gfc_error ("Attribute at %L is not allowed in a %s definition",
+ _at[d], state_name);
m = MATCH_ERROR;
goto cleanup;
}
Re: [Patch, fortran] PR93833 - [8/9/10 Regression] ICE in trans_array_constructor, at fortran/trans-array.c:2566
Unfortunately the mailing list stripped off this attachment so we do not have a chance to review. As attachments appear to be working lately, please resubmit this patch. Fritz On Sun, Mar 8, 2020 at 8:58 AM Paul Richard Thomas wrote: > > This is yet another case, where a deferred character length variable > loses the character length backend_decl. As previously, converting the > expression and using the string_length recovers it successfully. > > Regtested on FC31/x86_64 - OK for trunk, followed by 8- and 9-branches > after a week or two? > > Paul > > 2020-03-08 Paul Thomas > > PR fortran/93833 > * trans-array.c (get_array_ctor_var_strlen): If the character > length backend_decl cannot be found, convert the expression and > use the string length. Clear up some minor white space issues in > the rest of the file. > > 2020-03-08 Paul Thomas > > PR fortran/93833 > * gfortran.dg/deferred_character_36.f90 : New test.
Re: PATCH -- Fix degree trignometric functions
On Fri, Mar 27, 2020 at 7:36 PM Fritz Reese wrote:
>
> On Fri, Mar 6, 2020 at 6:18 PM Steve Kargl
> wrote:
> [...]
> > TL;DR version.
> >
> > Fix the simplification and handling of the degree trigonometric functions.
> > This includes fixing a number of ICEs. See PR 93871.
>
> An updated version of the patch is attached. Regression tests (and new
> test cases) are pending.
>
Patch with regression tests attached, and rebased onto master
(013fca64fc...). The new and updated tests cover several issues
discovered and discussed in PR 93871 at
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93871 . I am
bootstrapping and testing now. If everything passes, does this look OK
to commit?
gcc/fortran/ChangeLog:
PR fortran/93871
* gfortran.h (GFC_ISYM_ACOSD, GFC_ISYM_ASIND, GFC_ISYM_ATAN2D,
GFC_ISYM_ATAND, GFC_ISYM_COSD, GFC_ISYM_COTAND, GFC_ISYM_SIND,
GFC_ISYM_TAND): New.
* intrinsic.c (add_functions): Remove check for flag_dec_math.
Give degree trig functions simplification and name resolution
functions (e.g, gfc_simplify_atrigd () and gfc_resolve_atrigd ()).
(do_simplify): Remove special casing of degree trig functions.
* intrinsic.h (gfc_simplify_acosd, gfc_simplify_asind,
gfc_simplify_atand, gfc_simplify_cosd, gfc_simplify_cotand,
gfc_simplify_sind, gfc_simplify_tand, gfc_resolve_trigd2): Add new
prototypes.
(gfc_simplify_atrigd, gfc_simplify_trigd, gfc_resolve_cotan,
resolve_atrigd): Remove prototypes of deleted functions.
* iresolve.c (is_trig_resolved, copy_replace_function_shallow,
gfc_resolve_cotan, get_radians, get_degrees, resolve_trig_call,
gfc_resolve_atrigd, gfc_resolve_atan2d): Delete functions.
(gfc_resolve_trigd, gfc_resolve_trigd2): Resolve to library functions.
* simplify.c (rad2deg, deg2rad, gfc_simplify_acosd, gfc_simplify_asind,
gfc_simplify_atand, gfc_simplify_atan2d, gfc_simplify_cosd,
gfc_simplify_sind, gfc_simplify_tand, gfc_simplify_cotand): New
functions.
(gfc_simplify_atan2): Fix error message.
(simplify_trig_call, gfc_simplify_trigd, gfc_simplify_atrigd,
radians_f): Delete functions.
* trans-intrinsic.c: Add LIB_FUNCTION decls for sind, cosd, tand.
(rad2deg, gfc_conv_intrinsic_atrigd, gfc_conv_intrinsic_cotan,
gfc_conv_intrinsic_cotand, gfc_conv_intrinsic_atan2d): New functions.
(gfc_conv_intrinsic_function): Handle ACOSD, ASIND, ATAND, COTAN,
COTAND, ATAN2D.
* trigd_fe.inc: New file. Included by simplify.c to implement
simplify_sind, simplify_cosd, simplify_tand with code common to the
libgfortran implementation.
libgfortran/ChangeLog:
PR fortran/93871
* Makefile.am, Makefile.in: New make rule for intrinsics/trigd.c.
* gfortran.map: New routines for {sind, cosd, tand}X{r4, r8, r10, r16}.
* intrinsics/trigd.c, intrinsics/trigd_lib.inc, intrinsics/trigd.inc:
New files. Defines native degree-valued trig functions.
gcc/testsuite/ChangeLog:
PR fortran/93871
* gfortran.dg/dec_math.f90: Extend coverage to real(10) and real(16).
* gfortran.dg/dec_math_2.f90: New test.
* gfortran.dg/dec_math_3.f90: Likewise.
* gfortran.dg/dec_math_4.f90: Likewise.
* gfortran.dg/dec_math_5.f90: Likewise.
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 96037629f5f..371f77be64d 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -357,6 +357,7 @@ enum gfc_isym_id
GFC_ISYM_ACCESS,
GFC_ISYM_ACHAR,
GFC_ISYM_ACOS,
+ GFC_ISYM_ACOSD,
GFC_ISYM_ACOSH,
GFC_ISYM_ADJUSTL,
GFC_ISYM_ADJUSTR,
@@ -369,10 +370,13 @@ enum gfc_isym_id
GFC_ISYM_ANINT,
GFC_ISYM_ANY,
GFC_ISYM_ASIN,
+ GFC_ISYM_ASIND,
GFC_ISYM_ASINH,
GFC_ISYM_ASSOCIATED,
GFC_ISYM_ATAN,
GFC_ISYM_ATAN2,
+ GFC_ISYM_ATAN2D,
+ GFC_ISYM_ATAND,
GFC_ISYM_ATANH,
GFC_ISYM_ATOMIC_ADD,
GFC_ISYM_ATOMIC_AND,
@@ -410,8 +414,10 @@ enum gfc_isym_id
GFC_ISYM_CONJG,
GFC_ISYM_CONVERSION,
GFC_ISYM_COS,
+ GFC_ISYM_COSD,
GFC_ISYM_COSH,
GFC_ISYM_COTAN,
+ GFC_ISYM_COTAND,
GFC_ISYM_COUNT,
GFC_ISYM_CPU_TIME,
GFC_ISYM_CSHIFT,
@@ -598,6 +604,7 @@ enum gfc_isym_id
GFC_ISYM_SIGNAL,
GFC_ISYM_SI_KIND,
GFC_ISYM_SIN,
+ GFC_ISYM_SIND,
GFC_ISYM_SINH,
GFC_ISYM_SIZE,
GFC_ISYM_SLEEP,
@@ -618,6 +625,7 @@ enum gfc_isym_id
GFC_ISYM_SYSTEM,
GFC_ISYM_SYSTEM_CLOCK,
GFC_ISYM_TAN,
+ GFC_ISYM_TAND,
GFC_ISYM_TANH,
GFC_ISYM_TEAM_NUMBER,
GFC_ISYM_THIS_IMAGE,
diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c
index 3012187ddae..17f5efc6566 100644
--- a/gcc/fortran/intrinsic.c
+++ b/gcc/fortran/intrinsic.c
@@ -3281,116 +3281,130 @@ add_functions (void)
make_generic ("loc", GFC_ISYM_LOC, GFC_STD_GNU);
- if (flag_dec_math)
-{
- add_sym_1 ("acosd", GFC_ISYM_ACOS, CLASS_ELEMENTAL, ACTUAL_YES,
Re: PATCH -- Fix degree trignometric functions
On Mon, Mar 30, 2020 at 4:53 PM Tobias Burnus wrote: > > Hi Fritz, > > On 3/30/20 10:20 PM, Fritz Reese via Fortran wrote: > > >>> * All included files need dependency; I do not quickly > >>>see whether that' the case. > > If one looks at the build, the dependency is automatically > obtained and tracked in …/.deps/*.Po in the build directory. > Hence, no action needed. > > Cheers, > > Tobias Awesome, thanks! Fritz
Re: PATCH -- Fix degree trignometric functions
On Sat, Mar 28, 2020 at 12:37 PM Steve Kargl wrote: > > On Sat, Mar 28, 2020 at 08:10:38AM +0100, Tobias Burnus wrote: > > Two remarks: > > > > * As the file trigd_lib.inc is shared between libgfortran > > and gcc/fortran, I wonder whether it should be placed > > under include/ (under the GCC toplevel directroy) > > The original name and location were chosen to match > intrinsics/erfc_scaled_inc.c, which is included in > intrinsics/erfc_scaled.c. I think Fritz's re-use > in simplification makes since given the ugly nested > if-elseif-else constructs in the file. As this is > Fortran specific, I do not believe it should be moved > up to a toplevel diretory. I'm ambivalent about the location, though it is Fortran-specific. > > * All included files need dependency; I do not quickly > > see whether that' the case. Yes, I'd like to add a dependency for them as rebuilding after a change is a pain. I've not yet decoded the Make-fu involved, but I will be sure to add them for the final patch. Thanks for the feedback, Tobias. --- Fritz
Re: PATCH -- Fix degree trignometric functions
On Fri, Mar 6, 2020 at 6:18 PM Steve Kargl
wrote:
[...]
> TL;DR version.
>
> Fix the simplification and handling of the degree trigonometric functions.
> This includes fixing a number of ICEs. See PR 93871.
An updated version of the patch is attached. Regression tests (and new
test cases) are pending.
Changes since Steve's patch of note:
* libgfortran/intrinsics/trigd.c now indirectly includes trigd.inc
(formerly trigd_inc.c) through trigd_lib.inc. trigd.inc is now written
using GMP/MPFR functions, so that the same file can be included from
the front-end for simplification. The GMP/MPFR functions are
pre-processed into native code by trigd_lib.inc. This ensures that
both the FE and the library are using the same code for resolving
these functions, preventing future maintenance woes.
* TAND(90, 270) returns +Infinity and -Infinity, instead of NaN. This
is compatible with behavior of (at least some) DEC compilers.
* COTAN[D] are implemented as -TAN[D](x + 90 degrees) rather than 1 /
COTAN[D] for speed and to avoid singularities (though the new
implementation of TAND allows 1 / TAND to return zero when TAND
returns infinity).
* SMALL thresholds and signs of some specific values are corrected.
For REAL(10) and REAL(16), the SMALL threshold for SIND(x) = D2R(x) is
eliminated, since there would be loss of precision. The other
thresholds achieve COSD(x) = 1 and SIND(x) = D2R(x) without loss of
precision.
ChangeLogs are below. I will post an update soon after I perform
regression tests, which include some new tests which I will add.
gcc/fortran/ChangeLog:
PR fortran/93871
* gfortran.h (GFC_ISYM_ACOSD, GFC_ISYM_ASIND, GFC_ISYM_ATAN2D,
GFC_ISYM_ATAND, GFC_ISYM_COSD, GFC_ISYM_COTAND, GFC_ISYM_SIND,
GFC_ISYM_TAND): New.
* intrinsic.c (add_functions): Remove check for flag_dec_math.
Give degree trig functions simplification and name resolution
functions (e.g, gfc_simplify_atrigd () and gfc_resolve_atrigd ()).
(do_simplify): Remove special casing of degree trig functions.
* intrinsic.h (gfc_simplify_acosd, gfc_simplify_asind,
gfc_simplify_atand, gfc_simplify_cosd, gfc_simplify_cotand,
gfc_simplify_sind, gfc_simplify_tand, gfc_resolve_trigd2): Add new
prototypes.
(gfc_simplify_atrigd, gfc_simplify_trigd, gfc_resolve_cotan,
resolve_atrigd): Remove prototypes of deleted functions.
* iresolve.c (is_trig_resolved, copy_replace_function_shallow,
gfc_resolve_cotan, get_radians, get_degrees, resolve_trig_call,
gfc_resolve_atrigd, gfc_resolve_atan2d): Delete functions.
(gfc_resolve_trigd, gfc_resolve_trigd2): Resolve to library functions.
* simplify.c (rad2deg, deg2rad, gfc_simplify_acosd, gfc_simplify_asind,
gfc_simplify_atand, gfc_simplify_atan2d, gfc_simplify_cosd,
gfc_simplify_sind, gfc_simplify_tand, gfc_simplify_cotand): New
functions.
(gfc_simplify_atan2): Fix error message.
(simplify_trig_call, gfc_simplify_trigd, gfc_simplify_atrigd,
radians_f): Delete functions.
* trans-intrinsic.c: Add LIB_FUNCTION decls for sind, cosd, tand.
(rad2deg, gfc_conv_intrinsic_atrigd, gfc_conv_intrinsic_cotan,
gfc_conv_intrinsic_cotand, gfc_conv_intrinsic_atan2d): New functions.
(gfc_conv_intrinsic_function): Handle ACOSD, ASIND, ATAND, COTAN,
COTAND, ATAN2D.
* trigd_fe.inc: New file. Included by simplify.c to implement
simplify_sind, simplify_cosd, simplify_tand with code common to the
libgfortran implementation.
libgfortran/ChangeLog:
PR fortran/93871
* Makefile.am, Makefile.in: New make rule for intrinsics/trigd.c.
* gfortran.map: New routines for {sind, cosd, tand}X{r4, r8, r10, r16}.
* intrinsics/trigd.c, intrinsics/trigd_lib.inc, intrinsics/trigd.inc:
New files. Defines native degree-valued trig functions.
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 96037629f5f..371f77be64d 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -357,6 +357,7 @@ enum gfc_isym_id
GFC_ISYM_ACCESS,
GFC_ISYM_ACHAR,
GFC_ISYM_ACOS,
+ GFC_ISYM_ACOSD,
GFC_ISYM_ACOSH,
GFC_ISYM_ADJUSTL,
GFC_ISYM_ADJUSTR,
@@ -369,10 +370,13 @@ enum gfc_isym_id
GFC_ISYM_ANINT,
GFC_ISYM_ANY,
GFC_ISYM_ASIN,
+ GFC_ISYM_ASIND,
GFC_ISYM_ASINH,
GFC_ISYM_ASSOCIATED,
GFC_ISYM_ATAN,
GFC_ISYM_ATAN2,
+ GFC_ISYM_ATAN2D,
+ GFC_ISYM_ATAND,
GFC_ISYM_ATANH,
GFC_ISYM_ATOMIC_ADD,
GFC_ISYM_ATOMIC_AND,
@@ -410,8 +414,10 @@ enum gfc_isym_id
GFC_ISYM_CONJG,
GFC_ISYM_CONVERSION,
GFC_ISYM_COS,
+ GFC_ISYM_COSD,
GFC_ISYM_COSH,
GFC_ISYM_COTAN,
+ GFC_ISYM_COTAND,
GFC_ISYM_COUNT,
GFC_ISYM_CPU_TIME,
GFC_ISYM_CSHIFT,
@@ -598,6 +604,7 @@ enum gfc_isym_id
GFC_ISYM_SIGNAL,
GFC_ISYM_SI_KIND,
GFC_ISYM_SIN,
+ GFC_ISYM_SIND,
GFC_ISYM_SINH,
GFC_ISYM_SIZE,
GFC_ISYM_SLEEP,
