Re: Mattermost discussion on PR98426

[Bernhard Reutner-Fischer]

Hi Jerry, all!

Just a quick comment for that area.

On Thu, 25 Apr 2024 20:00:24 -0700
Jerry D  wrote:

> Hi posted some thoughts on the subject at our mattermost workspace.
> 
> This particular PR caught my attention because I have done things like 
> this before. I am looking forward to gcc15. I think changing things at 
> this point might be a bit intrusive.

Agree, clearly gcc-15 material.

> 
> I am thinking about:
> 
> "The FNV-1a algorithm is:
> 
> hash = FNV_offset_basis
> for each octetOfData to be hashed
>  hash = hash xor octetOfData
>  hash = hash * FNV_prime
> return hash"
> 
> Found here:
> 
> https://softwareengineering.stackexchange.com/questions/49550/which-hashing-algorithm-is-best-for-uniqueness-and-speed
> 
> If you are interested, comment on MatterMost.
> 
> Jerry -

As you may remember i was looking into using the stringpool for names
in the frontend. The plan was to
1) use the stringpool for names
2) Use a hashmap (or some suitable data structure) to lookup names
instead of lots of strcmp().

I posted patches for 1) to that effect here in this thread:
(I)
https://inbox.sourceware.org/fortran/20230413230440.229ebc2f@nbbrfq/

AFAIR part 1) was complete. But the (only) tricky part of 1) involved
module symbols, as mentioned in the link above.
Specifically "[PATCH,FORTRAN v2] Use stringpool on loading module
symbols": 
https://inbox.sourceware.org/gcc-patches/20180919225533.20009-1-rep.dot@gmail.com/
I described the problems i was seeing with module writing,
free_pi_tree (around true_name), finalization wrapper vars and
names of fixups in the mail (I). Unfortunately this would involve a
bump of the module version. Furthermore this patch was not reviewed
yet, so if you have comments, i'm all ears :) Thoughts?

There in (I) you can also see why i think that using attr.artificial is
the way to go. Later i also sent a patch (that was also not applied
yet) to additionally tweak class component and such internally generated
symbols to be marked as artificial:
https://inbox.sourceware.org/fortran/2024231748.376086cd@nbbrfq/
Thoughts?

It would be nice if we could finally tackle this for gcc-15, IMHO.

PS: unfortunately the git branch (
https://gcc.gnu.org/git/?p=gcc.git;a=log;h=refs/heads/aldot/fortran-fe-stringpool
 ) mentioned in mail (I) lived in the old git, before we switched to
git, and i fear that branch maybe got lost. But i strongly believe
that all patches of the fortran-fe-stringpool branch can be found in
the thread mentioned above.

HTH

Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat

[Bernhard Reutner-Fischer]

On Wed, 28 Feb 2024 21:29:06 -0800
Jerry D  wrote:

> The attached patch adds the error checks similar to the first patch 
> previously committed.
> 
> I noticed a redundancy in some defines MSGLEN and IOMSG_LEN so I 
> consolidated this to one define in io.h. This is just cleanup stuff.
> 
> I have added test cases for each of the places where UDTIO is done in 
> the library.
> 
> Regressions tested on x86_64.
> 
> OK for trunk?

I think the commit hooks will complain about several missing spaces
before open brace; See contrib/check_GNU_style.py /tmp/pr105456-3.diff

Would it make sense to introduce and use an internal helper like trim()?
Or would it be possible to trim the message in generate_error_common()?

And, just for my own education, the length limitation of iomsg to 255
chars is not backed by the standard AFAICS, right? It's just our
STRERR_MAXSZ?

thanks!

> 
> Regards,
> 
> Jerry
> 
> commit 640991bd6b83df4197b2eaec63d1e0e695e48b75
> Author: Jerry DeLisle 
> Date:   Wed Feb 28 20:51:06 2024 -0800
> 
>  Fortran: Add user defined error messages for UDTIO.
> 
>  The defines IOMSG_LEN and MSGLEN were redundant so these are combined
>  into IOMSG_LEN as defined in io.h.
> 
>  The remainder of the patch adds checks for when a user defined
>  derived type IO procedure sets the IOSTAT or IOMSG variables
>  independent of the librrary defined I/O messages.
> 
>  PR libfortran/105456
> 
>  libgfortran/ChangeLog:
> 
>  * io/io.h (IOMSG_LEN): Moved to here.
>  * io/list_read.c (MSGLEN): Removed MSGLEN.
>  (convert_integer): Changed MSGLEN to IOMSG_LEN.
>  (parse_repeat): Likewise.
>  (read_logical): Likewise.
>  (read_integer): Likewise.
>  (read_character): Likewise.
>  (parse_real): Likewise.
>  (read_complex): Likewise.
>  (read_real): Likewise.
>  (check_type): Likewise.
>  (list_formatted_read_scalar): Adjust to IOMSG_LEN.
>  (nml_read_obj): Add user defined error message.
>  * io/transfer.c (unformatted_read): Add user defined error
>  message.
>  (unformatted_write): Add user defined error message.
>  (formatted_transfer_scalar_read): Add user defined error 
> message.
>  (formatted_transfer_scalar_write): Add user defined error 
> message.
>  * io/write.c (list_formatted_write_scalar): Add user 
> defined error message.
>  (nml_write_obj): Add user defined error message.
> 
>  gcc/testsuite/ChangeLog:
> 
>  * gfortran.dg/pr105456-nmlr.f90: New test.
>  * gfortran.dg/pr105456-nmlw.f90: New test.
>  * gfortran.dg/pr105456-ruf.f90: New test.
>  * gfortran.dg/pr105456-wf.f90: New test.
>  * gfortran.dg/pr105456-wuf.f90: New test.

Re: [PATCH] Fortran: Mark internal symbols as artificial [PR88009,PR68800]

[Bernhard Reutner-Fischer]

On Wed, 17 Nov 2021 21:32:14 +0100
Harald Anlauf  wrote:

> Do you have testcases/reproducers demonstrating that the patch actually
> fixes the issues you're describing?

I believe that marking artificial symbols as such is obvious and i did
use the existing tests to verify that the changes do not regress but
behave as intended. I did check that the memory leak in
gfc_find_derived_vtab is fixed with the patch.

Ok for stage 1 if the rebased regression test passes?

thanks

> 
> Am 17.11.21 um 09:12 schrieb Bernhard Reutner-Fischer via Gcc-patches:
> > On Tue, 16 Nov 2021 21:46:32 +0100
> > Harald Anlauf via Fortran  wrote:
> >  
> >> Hi Bernhard,
> >>
> >> I'm trying to understand your patch.  What does it really try to solve?  
> >
> > Compiler generated symbols should be marked artificial.
> > The fix for PR88009 ( f8add009ce300f24b75e9c2e2cc5dd944a020c28 ,
> > r9-5194 ) added artificial just to the _final component and left out all 
> > the rest.
> > Note that the majority of compiler generated symbols in class.c
> > already had artificial set properly.
> > The proposed patch amends the other generated symbols to be marked
> > artificial, too.
> >
> > The other parts fix memory leaks.
> >  
> >>
> >> PR88009 is closed and seems to have nothing to do with this.  
> >
> > Well it marked only _final as artificial and forgot to adjust the
> > others as well.
> > We can remove the reference to PR88009 if you prefer?
> >
> > thanks!  
> >>
> >> Harald
> >>
> >> Am 14.11.21 um 23:17 schrieb Bernhard Reutner-Fischer via Fortran:  
> >>> Hi!
> >>>
> >>> Amend fix for PR88009 to mark all these class components as artificial.
> >>>
> >>> gcc/fortran/ChangeLog:
> >>>
> >>>   * class.c (gfc_build_class_symbol, 
> >>> generate_finalization_wrapper,
> >>>   (gfc_find_derived_vtab, find_intrinsic_vtab): Use stringpool for
> >>>   names. Mark internal symbols as artificial.
> >>>   * decl.c (gfc_match_decl_type_spec, gfc_match_end): Fix
> >>>   indentation.
> >>>   (gfc_match_derived_decl): Fix indentation. Check extension level
> >>>   before incrementing refs counter.
> >>>   * parse.c (parse_derived): Fix style.
> >>>   * resolve.c (resolve_global_procedure): Likewise.
> >>>   * symbol.c (gfc_check_conflict): Do not ignore artificial 
> >>> symbols.
> >>>   (gfc_add_flavor): Reorder condition, cheapest first.
> >>>   (gfc_new_symbol, gfc_get_sym_tree,
> >>>   generate_isocbinding_symbol): Fix style.
> >>>   * trans-expr.c (gfc_trans_subcomponent_assign): Remove
> >>>   restriction on !artificial.
> >>>   * match.c (gfc_match_equivalence): Special-case CLASS_DATA for
> >>>   warnings.
> >>>
> >>> ---
> >>> gfc_match_equivalence(), too, should not bail-out early on the first
> >>> error but should diagnose all errors. I.e. not goto cleanup but set
> >>> err=true and continue in order to diagnose all constraints of a
> >>> statement. Maybe Sandra or somebody else will eventually find time to
> >>> tweak that.
> >>>
> >>> I think it also plugs a very minor leak of name in gfc_find_derived_vtab
> >>> so i also tagged it [PR68800]. At least that was the initial
> >>> motiviation to look at that spot.
> >>> We were doing
> >>> -  name = xasprintf ("__vtab_%s", tname);
> >>> ...
> >>> gfc_set_sym_referenced (vtab);
> >>> - name = xasprintf ("__vtype_%s", tname);
> >>>
> >>> Bootstrapped and regtested without regressions on x86_64-unknown-linux.
> >>> Ok for trunk?
> >>>  
> >>
> >>  
> >
> >  
> 

Re: [PATCH] Fortran: annotations for DO CONCURRENT loops [PR113305]

[Bernhard Reutner-Fischer]

On Wed, 10 Jan 2024 23:24:22 +0100
Harald Anlauf  wrote:

> diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
> index 82f388c05f8..88502c1e3f0 100644
> --- a/gcc/fortran/gfortran.h
> +++ b/gcc/fortran/gfortran.h
> @@ -2926,6 +2926,10 @@ gfc_dt;
>  typedef struct gfc_forall_iterator
>  {
>gfc_expr *var, *start, *end, *stride;
> +  unsigned short unroll;
> +  bool ivdep;
> +  bool vector;
> +  bool novector;
>struct gfc_forall_iterator *next;
>  }
[]
> diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc
> index a718dce237f..59a9cf99f9b 100644
> --- a/gcc/fortran/trans-stmt.cc
> +++ b/gcc/fortran/trans-stmt.cc
> @@ -41,6 +41,10 @@ typedef struct iter_info
>tree start;
>tree end;
>tree step;
> +  unsigned short unroll;
> +  bool ivdep;
> +  bool vector;
> +  bool novector;
>struct iter_info *next;
>  }

Given that we already have in gfortran.h

> typedef struct
> {
>   gfc_expr *var, *start, *end, *step;
>   unsigned short unroll;
>   bool ivdep;
>   bool vector;
>   bool novector;
> }
> gfc_iterator;

would it make sense to break out these loop annotation flags into its
own let's say struct gfc_iterator_flags and use pointers to that flags
instead?

LGTM otherwise.
Thanks for the patch!

Re: [PATCH v4] libgfortran: Replace mutex with rwlock

[Bernhard Reutner-Fischer]

[CCing Ian as libgcc maintainer]

On Wed, 1 Nov 2023 10:14:37 +
"Zhu, Lipeng"  wrote:

> > >
> > > Hi Lipeng,
> > >  
> > > >>> Sure, as your comments, in the patch V6, I added 3 test cases with
> > > >>> OpenMP to test different cases in concurrency respectively:
> > > >>> 1. find and create unit very frequently to stress read lock and write 
> > > >>> lock.
> > > >>> 2. only access the unit which exist in cache to stress read lock.
> > > >>> 3. access the same unit in concurrency.
> > > >>> For the third test case, it also help to find a bug:  When unit
> > > >>> can't be found in cache nor unit list in read phase, then threads
> > > >>> will try to acquire write lock to insert the same unit, this will
> > > >>> cause duplicate key  
> > > >> error.  
> > > >>> To fix this bug, I get the unit from unit list once again before
> > > >>> insert in write  
> > > >> lock.  
> > > >>> More details you can refer the patch v6.
> > > >>>  
> > > >>
> > > >> Could you help to review this update? I really appreciate your 
> > > >> assistance.
> > > >>  
> > >  
> > > > Could you help to review this update?  Any concern will be appreciated. 
> > > >  
> > >
> > > Fortran parts are OK (I think I wrote that already), we need somebody
> > > for the non-Fortran parts.
> > >  
> > Hi Thomas,
> > 
> > Thanks for your response. Very appreciate for your patience and help.
> >   
> > > Jakub, could you maybe take a look?
> > >
> > > Best regards
> > >
> > >   Thomas  
> > 
> > Hi Jakub,
> > 
> > Can you help to take a look at the change for libgcc part that added several
> > rwlock macros in libgcc/gthr-posix.h?
> >   
> 
> Hi Jakub,
> 
> Could you help to review this, any comment will be greatly appreciated.

Latest version is at
https://inbox.sourceware.org/gcc-patches/20230818031818.2161842-1-lipeng@intel.com/

> 
> > Best Regards,
> > Lipeng Zhu  
> 

Re: [PATCH] fortran: error recovery on duplicate declaration of class variable [PR95710]

[Bernhard Reutner-Fischer via Fortran]

On 22 September 2023 21:16:35 CEST, Harald Anlauf via Fortran 
 wrote:
>Dear all,
>
>the attached simple and obvious patch fixes several NULL pointer
>dereferences that are encountered for a duplicate declaration of
>a class variable.  Another one from Gerhard's torture tests...

Obviously ok.
thanks,

>
>Regtested on x86_64-pc-linux-gnu.
>
>I intend to commit within 24h unless there are comments.
>
>Thanks,
>Harald
>

Re: [PATCH 8/8] OpenMP: Fortran "!$omp declare mapper" support

[Bernhard Reutner-Fischer via Fortran]

On Tue, 5 Sep 2023 12:28:28 -0700
Julian Brown  wrote:

> +  static bool
> +  equal (const omp_name_type ,
> +  const omp_name_type )
> +  {
> +if (a.name == NULL_TREE && b.name == NULL_TREE)
> +  return a.type == b.type;

I'm curious if (and why) the type comparison above is safe and does not
use gfc_compare_types () ?

thanks,

> +else if (a.name == NULL_TREE || b.name == NULL_TREE)
> +  return false;
> +else
> +  return a.name == b.name && gfc_compare_types (a.type, b.type);
> +  }

Re: [PATCH] Introduce hardbool attribute for C

[Bernhard Reutner-Fischer via Fortran]

On Wed, 21 Jun 2023 22:08:55 -0300
Alexandre Oliva  wrote:

> Thanks for the test.
> 
> Did you mean for me to incorporate it into the patch, or do you mean to
> contribute it separately, if the feature happens to be accepted?

These were your tests that i quoted but i or my MUA botched to add one
level of quotes -- sorry for that.

> 
> On Jun 19, 2023, Bernhard Reutner-Fischer  wrote:
> 
> > I don't see explicit tests with _Complex nor __complex__. Would we
> > want to check these here, or are they handled thought the "underlying"
> > tests above?  
> 
> Good question.  The notion of using complex types to hold booleans
> hadn't even crossed my mind.

Maybe it is not real, it just sparkled through somehow.

> On the one hand, there doesn't seem to be reason to rule them out, and
> that could go for literally any other type.
> 
> On the other, there doesn't seem to be any useful case for them.  Can
> anyone think of one?

We could either not reject other such uses and wait or we could reject
them and equally wait for complaints. I would not dare to bet who pops
up first, fuzzers or users, though arguments of the latter would
probably be interesting.. I don't have an opinion (nor a use-case),
really, it was just a thought (i mentioned tinfoil hat, did i ;).

> 
> > I'd welcome a fortran interop note in the docs  
> 
> Is there any good place for such interop notes?  I'm not sure I'm
> best-suited to write them up, since Fortran is not a language I'm
> very familiar with, but I suppose I could give it a try.
> 

I'd append to your extend.texi hunk below the para about uninitialized a
note to the effect of:
Note: Types annotated with this attribute may not be Fortran
interoperable.

I would not go into too much detail about C_BOOL nor LOGICAL for i
reckon anybody sensibilised to either two of that attribute, C and
Fortran will draw her conclusions.
Didn't really think how easy it would be to handle this on the user
side, but i fear the modern iso_c_binding way would need help from the
compiler for the lazy. I'd expect a user to be able to trivially
translate this in wrappers done the old way though, which is a pity
from an educational and modernisation POV. Didn't look closely, so this
guesstimate might be all wrong, of course.

thanks,

Re: [Patch, fortran] PR107900 Select type with intrinsic type inside associate causes ICE / Segmenation fault

[Bernhard Reutner-Fischer via Fortran]

Hi!

First of all, many thanks for the patch!
If i may, i have a question concerning the chosen style in the error
message and one nitpick concerning a return type though, the latter
primarily prompting this reply.

On Tue, 20 Jun 2023 11:54:25 +0100
Paul Richard Thomas via Fortran  wrote:

> diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
> index d5cfbe0cc55..c960dfeabd9 100644
> --- a/gcc/fortran/expr.cc
> +++ b/gcc/fortran/expr.cc

> @@ -6470,6 +6480,22 @@ gfc_check_vardef_context (gfc_expr* e, bool pointer, 
> bool alloc_obj,
>   }
> return false;
>   }
> +  else if (context && gfc_is_ptr_fcn (assoc->target))
> + {
> +   if (!gfc_notify_std (GFC_STD_F2018, "%qs at %L associated to "
> +"pointer function target being used in a "
> +"variable definition context (%s)", name,
> +>where, context))

I'm curious why you decided to put context in braces and not simply use
quotes as per %qs?

> + return false;
> +   else if (gfc_has_vector_index (e))
> + {
> +   gfc_error ("%qs at %L associated to vector-indexed target"
> +  " cannot be used in a variable definition"
> +  " context (%s)",
> +  name, >where, context);

Ditto.

> diff --git a/gcc/fortran/match.cc b/gcc/fortran/match.cc
> index e7be7fddc64..0e4b5440393 100644
> --- a/gcc/fortran/match.cc
> +++ b/gcc/fortran/match.cc
> @@ -6377,6 +6377,39 @@ build_class_sym:
>  }
> 
> 
> +/* Build the associate name  */
> +static int
> +build_associate_name (const char *name, gfc_expr **e1, gfc_expr **e2)
> +{

> +return 1;

> +  return 0;
> +}

I've gone through the frontend recently and changed several such
boolean functions to use bool where appropriate. May i ask folks to use
narrower types in new code, please?
Iff later in the pipeline it is considered appropriate or benefical to
promote types, these will eventually be promoted.

> diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
> index e6a4337c0d2..18589e17843 100644
> --- a/gcc/fortran/trans-decl.cc
> +++ b/gcc/fortran/trans-decl.cc

> @@ -1906,6 +1915,7 @@ gfc_get_symbol_decl (gfc_symbol * sym)
>   gcc_assert (!sym->value || sym->value->expr_type == EXPR_NULL);
>  }
> 
> +

ISTM that the addition of vertical whitespace like here is in
contradiction with the coding style.

Please kindly excuse my comment and, again, thanks!

>gfc_finish_var_decl (decl, sym);
> 
>if (sym->ts.type == BT_CHARACTER)

Re: [PATCH] Introduce hardbool attribute for C

[Bernhard Reutner-Fischer via Fortran]

On 16 June 2023 07:35:27 CEST, Alexandre Oliva via Gcc-patches 
 wrote:

index 0..634feaed4deef
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/hardbool-err.c
@@ -0,0 +1,28 @@
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+typedef _Bool __attribute__ ((__hardbool__))
+hbbl; /* { dg-error "integral types" } */
+
+typedef double __attribute__ ((__hardbool__))
+hbdbl; /* { dg-error "integral types" } */
+
+enum x;
+typedef enum x __attribute__ ((__hardbool__))
+hbenum; /* { dg-error "integral types" } */
+
+struct s;
+typedef struct s __attribute__ ((__hardbool__))
+hbstruct; /* { dg-error "integral types" } */
+
+typedef int __attribute__ ((__hardbool__ (0, 0)))
+hb00; /* { dg-error "different values" } */
+
+typedef int __attribute__ ((__hardbool__ (4, 16))) hb4x;
+struct s {
+ hb4x m:2;
+}; /* { dg-error "is a GCC extension|different values" } */
+/* { dg-warning "changes value" "warning" { target *-*-* } .-1 } */
+
+hb4x __attribute__ ((vector_size (4 * sizeof (hb4x
+vvar; /* { dg-error "invalid vector type" } */

Arm-chair, tinfoil hat still on, didn't look closely, hence:

I don't see explicit tests with _Complex nor __complex__. Would we want to 
check these here, or are they handled thought the "underlying" tests above?

I'd welcome a fortran interop note in the docs as hinted previously to cover 
out of the box behavior. It's probably reasonably unlikely but better be safe 
than sorry?
cheers,

Re: Possible funding of gfortran work

[Bernhard Reutner-Fischer via Fortran]

On 14 June 2023 11:40:06 CEST, Mikael Morin  wrote:

>> What do you prefer? "not affiliated" or
>> "private", ...?
>> 
>Yes, that's fine.  Nothing.  Whatever.

IMHO that usually would be not applicable / n/a

Re: Possible funding of gfortran work

[Bernhard Reutner-Fischer via Fortran]

On 1 June 2023 11:18:08 CEST, Andre Vehreschild via Fortran 
 wrote:
>Hi Damian, all,
>
>thank you for your input. I have incorporated most of it. Due to Germany
>stepping out of nuclear use, I have reduced the cites on these to a minimum. I
>don't know anything about the people evaluating the proposal and don't want to
>be rejected just because of ideological reasons. Here is the proposal so far.

Good point.
There must be a ton fortran code running near ITER, maybe someone knows if any 
of that uses coarrays or would be inclined to do so if it would be improved?

Just a thought..

Re: Possible funding of gfortran work

[Bernhard Reutner-Fischer via Fortran]

On 26 May 2023 06:34:52 CEST, Jerry DeLisle via Fortran  
wrote:

>Maybe we can get someone to push forward on te native coarrays work?

It would be nice if someone could streamline the locking therein, as said.

Re: [PATCH 08/14] fortran: use _P() defines from tree.h

[Bernhard Reutner-Fischer via Fortran]

On Thu, 18 May 2023 21:20:41 +0200
Mikael Morin  wrote:

> Le 18/05/2023 à 17:18, Bernhard Reutner-Fischer a écrit :

> > I've fed gfortran.h into the script and found some CLASS_DATA spots,
> > see attached bootstrapped and tested patch.
> > Do we want to have that?  
> Some of it makes sense, but not all of it.
> 
> It is a macro to access the _data component of a class container.
> So for class-related stuff it makes sense to use CLASS_DATA, and 
> typically there will be a check that the type is BT_CLASS before.
> But for cases where we loop over all of the components of a type that is 
> not necessarily a class container, it doesn't make sense to use CLASS_DATA.
> 
> So I suggest to only keep the following hunks.
[]
> OK for those hunks.

Pushed those as r14-1001-g05b7cc7daac8b3
Many thanks!

PS: I'm attaching the fugly script i used to do these macro
replacements FYA.


use-defines.1.awk
Description: application/awk

Re: [PATCH v2] Fortran: Narrow return types [PR78798]

[Bernhard Reutner-Fischer via Fortran]

On Sun, 14 May 2023 14:27:42 +0200
Mikael Morin  wrote:

> Le 10/05/2023 à 18:47, Bernhard Reutner-Fischer via Fortran a écrit :
> > From: Bernhard Reutner-Fischer 
> > 
> > gcc/fortran/ChangeLog:
> > 
> > PR fortran/78798
> > * array.cc (compare_bounds): Use narrower return type.
> > (gfc_compare_array_spec): Likewise.
> > (is_constant_element): Likewise.
> > (gfc_constant_ac): Likewise.  
> (...)
> > ---
> > Bootstrapped without new warnings and regression tested on
> > x86_64-linux with no regressions, OK for trunk?
> >   
> (...)
> > diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc
> > index b348bda6e6c..4e3aed84b9d 100644
> > --- a/gcc/fortran/check.cc
> > +++ b/gcc/fortran/check.cc
> > @@ -1156,7 +1156,7 @@ dim_rank_check (gfc_expr *dim, gfc_expr *array, int 
> > allow_assumed)
> >  dimension bi, returning 0 if they are known not to be identical,
> >  and 1 if they are identical, or if this cannot be determined.  */
> >   
> > -static int
> > +static bool
> >   identical_dimen_shape (gfc_expr *a, int ai, gfc_expr *b, int bi)
> >   {
> > mpz_t a_size, b_size;  
> 
> To be consistent, please change as well the local variable "ret" used as 
> return value from int to bool.
> 
> > diff --git a/gcc/fortran/cpp.cc b/gcc/fortran/cpp.cc
> > index c3b7c7f7bd9..d7890a97287 100644
> > --- a/gcc/fortran/cpp.cc
> > +++ b/gcc/fortran/cpp.cc
> > @@ -297,7 +297,7 @@ gfc_cpp_init_options (unsigned int 
> > decoded_options_count,
> > gfc_cpp_option.deferred_opt_count = 0;
> >   }
> >   
> > -int
> > +bool
> >   gfc_cpp_handle_option (size_t scode, const char *arg, int value 
> > ATTRIBUTE_UNUSED)
> >   {
> > int result = 1;  
> 
> Same here, change the type of variable "result".
> 
> (...)
> > diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
> > index a648d5c7903..b398b29a642 100644
> > --- a/gcc/fortran/dependency.cc
> > +++ b/gcc/fortran/dependency.cc  
> (...)
> 
> > @@ -1091,7 +1091,7 @@ gfc_check_argument_dependency (gfc_expr *other, 
> > sym_intent intent,
> >   /* Like gfc_check_argument_dependency, but check all the arguments in 
> > ACTUAL.
> >  FNSYM is the function being called, or NULL if not known.  */
> >   
> > -int
> > +bool
> >   gfc_check_fncall_dependency (gfc_expr *other, sym_intent intent,
> >  gfc_symbol *fnsym, gfc_actual_arglist *actual,
> >  gfc_dep_check elemental)  
> 
> Why not change the associated subfunctions 
> (gfc_check_argument_dependency, gfc_check_argument_var_dependency) as well ?

I have left these subfunctions alone for now to get the other hunks out
of the way. I have adjusted the patch according to your other comments
and pushed it as r14-973-gc072df1ab14450.

Thanks!

> 
> (...)
> > @@ -2098,7 +2098,7 @@ ref_same_as_full_array (gfc_ref *full_ref, gfc_ref 
> > *ref)
> > there is some kind of overlap.
> > 0 : array references are identical or not overlapping.  */
> >   
> > -int
> > +bool
> >   gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse,
> >   bool identical)
> >   {  
> 
> The function comment states that the function may return 2, which 
> doesn't seem to be the case any more.  So please update the comment.
> 
> (...)> diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc
> > index 221165d6dac..b4b36e27d75 100644
> > --- a/gcc/fortran/symbol.cc
> > +++ b/gcc/fortran/symbol.cc
> > @@ -3216,7 +3216,7 @@ gfc_find_symtree_in_proc (const char* name, 
> > gfc_namespace* ns)
> >  any parent namespaces if requested by a nonzero parent_flag.
> >  Returns nonzero if the name is ambiguous.  */
> >   
> > -int
> > +bool
> >   gfc_find_sym_tree (const char *name, gfc_namespace *ns, int parent_flag,
> >gfc_symtree **result)
> >   {  
> 
> Maybe change nonzero to true in the comment?
> 
> (...)
> 
> OK with all the above fixed.
> 
> Thanks.
> 

Re: [PATCH 08/14] fortran: use _P() defines from tree.h

[Bernhard Reutner-Fischer via Fortran]

On Sun, 14 May 2023 15:10:12 +0200
Mikael Morin  wrote:

> Le 14/05/2023 à 01:23, Bernhard Reutner-Fischer via Gcc-patches a écrit :
> > From: Bernhard Reutner-Fischer 
> > 
> > gcc/fortran/ChangeLog:
> > 
> > * trans-array.cc (is_pointer_array): Use _P() defines from tree.h.
> > (gfc_conv_scalarized_array_ref): Ditto.
> > (gfc_conv_array_ref): Ditto.
> > * trans-decl.cc (gfc_finish_decl): Ditto.
> > (gfc_get_symbol_decl): Ditto.
> > * trans-expr.cc (gfc_trans_pointer_assignment): Ditto.
> > (gfc_trans_arrayfunc_assign): Ditto.
> > (gfc_trans_assignment_1): Ditto.
> > * trans-intrinsic.cc (gfc_conv_intrinsic_minmax): Ditto.
> > (conv_intrinsic_ieee_value): Ditto.
> > * trans-io.cc (gfc_convert_array_to_string): Ditto.
> > * trans-openmp.cc (gfc_omp_is_optional_argument): Ditto.
> > (gfc_trans_omp_clauses): Ditto.
> > * trans-stmt.cc (gfc_conv_label_variable): Ditto.
> > * trans.cc (gfc_build_addr_expr): Ditto.
> > (get_array_span): Ditto.  
> 
> OK from the fortran side.
> 
> Thanks

Thanks, i'll push it during the weekend.

I've fed gfortran.h into the script and found some CLASS_DATA spots,
see attached bootstrapped and tested patch.
Do we want to have that?
If so, i'd write a proper ChangeLog, of course.

Thanks!
diff --git a/gcc/fortran/class.cc b/gcc/fortran/class.cc
index 9d0c802b867..1466b07e260 100644
--- a/gcc/fortran/class.cc
+++ b/gcc/fortran/class.cc
@@ -889,7 +889,7 @@ copy_vtab_proc_comps (gfc_symbol *declared, gfc_symbol *vtype)
 
   vtab = gfc_find_derived_vtab (declared);
 
-  for (cmp = vtab->ts.u.derived->components; cmp; cmp = cmp->next)
+  for (cmp = CLASS_DATA (vtab); cmp; cmp = cmp->next)
 {
   if (gfc_find_component (vtype, cmp->name, true, true, NULL))
 	continue;
@@ -1078,7 +1078,7 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp,
   gfc_component *c;
 
   vtab = gfc_find_derived_vtab (comp->ts.u.derived);
-  for (c = vtab->ts.u.derived->components; c; c = c->next)
+  for (c = CLASS_DATA (vtab); c; c = c->next)
 	if (strcmp (c->name, "_final") == 0)
 	  break;
 
@@ -1143,7 +1143,7 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp,
 {
   gfc_component *c;
 
-  for (c = comp->ts.u.derived->components; c; c = c->next)
+  for (c = CLASS_DATA (comp); c; c = c->next)
 	finalize_component (e, comp->ts.u.derived, c, stat, fini_coarray, code,
 			sub_ns);
   gfc_free_expr (e);
@@ -1675,7 +1675,7 @@ generate_finalization_wrapper (gfc_symbol *derived, gfc_namespace *ns,
   gfc_component *comp;
 
   vtab = gfc_find_derived_vtab (derived->components->ts.u.derived);
-  for (comp = vtab->ts.u.derived->components; comp; comp = comp->next)
+  for (comp = CLASS_DATA (vtab); comp; comp = comp->next)
 	if (comp->name[0] == '_' && comp->name[1] == 'f')
 	  {
 	ancestor_wrapper = comp->initializer;
@@ -2752,7 +2752,7 @@ yes:
 {
   /* Return finalizer expression.  */
   gfc_component *final;
-  final = vtab->ts.u.derived->components->next->next->next->next->next;
+  final = CLASS_DATA (vtab)->next->next->next->next->next;
   gcc_assert (strcmp (final->name, "_final") == 0);
   gcc_assert (final->initializer
 		  && final->initializer->expr_type != EXPR_NULL);
diff --git a/gcc/fortran/data.cc b/gcc/fortran/data.cc
index d29eb12c1b1..f907bb35eb1 100644
--- a/gcc/fortran/data.cc
+++ b/gcc/fortran/data.cc
@@ -730,7 +730,7 @@ formalize_structure_cons (gfc_expr *expr)
   if (!cur || cur->n.component == NULL)
 return;
 
-  for (order = expr->ts.u.derived->components; order; order = order->next)
+  for (order = CLASS_DATA (expr); order; order = order->next)
 {
   cur = find_con_by_component (order, expr->value.constructor);
   if (cur)
diff --git a/gcc/fortran/dependency.cc b/gcc/fortran/dependency.cc
index b398b29a642..864470afdec 100644
--- a/gcc/fortran/dependency.cc
+++ b/gcc/fortran/dependency.cc
@@ -1253,7 +1253,7 @@ check_data_pointer_types (gfc_expr *expr1, gfc_expr *expr2)
 
   if (sym1->ts.type == BT_DERIVED && !seen_component_ref)
 {
-  for (cm1 = sym1->ts.u.derived->components; cm1; cm1 = cm1->next)
+  for (cm1 = CLASS_DATA (sym1); cm1; cm1 = cm1->next)
 	{
 	  if (cm1->ts.type == BT_DERIVED)
 	return false;
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index aa01a4d3d22..a6b4ef0a0bf 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -2671,7 +2671,7 @@ check_alloc_comp_init (gfc_expr *e)
   gcc_assert (e->expr_type == EXPR_STRUCTURE);
   gcc_assert (e->ts.type == BT_DERIVED || e->ts.type 

Re: [PATCH v2] Fortran: Narrow return types [PR78798]

[Bernhard Reutner-Fischer via Fortran]

On Sun, 14 May 2023 15:04:15 +0200
Thomas Koenig  wrote:

> On 14.05.23 14:27, Mikael Morin wrote:
> > 
> > (...)  
> >> @@ -2098,7 +2098,7 @@ ref_same_as_full_array (gfc_ref *full_ref, 
> >> gfc_ref *ref)
> >>   there is some kind of overlap.
> >>   0 : array references are identical or not overlapping.  */
> >> -int
> >> +bool
> >>   gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse,
> >>     bool identical)
> >>   {  
> > 
> > The function comment states that the function may return 2, which 
> > doesn't seem to be the case any more.  
> 
> Hm, this makes me a litte suspicious.  Was functionality for reversing
> loops lost,  maybe unintentionally?  I assume that, at some time, we did
> use the '2' as return value (or did we?)

There was 7c428aa29d75ef163c334cf3974f87b3630d8b8b (a revert because it
miscompiled spec2k) which might have associated the comment of the
former static gfc_dependency dep_ref (gfc_ref *lref, gfc_ref *rref,
gfc_reverse *reverse) to the current gfc_dep_resolver.

The commit which introduced the return value 2 documentation was
3d03ead0b8273efde57f6194617b35111a84b05d 
"re PR fortran/24524 (Fortran dependency checking should reverse loops)"

but TBH i don't see how it returned 2 in that revision?
Looks like when writing that patch it deemed useful to return 2 for
this specific situation but in the end it was dropped but the comment
survived.

I will update the comment to document the true / false return values.

And Mikael, do you want me to cleanup 1/0 to true/false assignments for
the boolean variables, or can we do that in a separate patch (or not at
all right now)?

many thanks for the reviews!

Re: [PATCH v2 4/7] fortran: use grep instead of fgrep

[Bernhard Reutner-Fischer via Fortran]

On Mon, 27 Jun 2022 14:10:36 +0800
Xi Ruoyao  wrote:

> fgrep has been deprecated in favor of grep -F for a long time, and the
> next grep release (3.8 or 4.0) will print a warning of fgrep is used.
> Stop using fgrep so we won't see the warning.
> 
> We can't hard code grep -F here or it may break build on hosts w/o GNU
> grep.  autoconf documentation contains a warning about this issue and
> suggest to use AC_PROG_FGREP and $FGREP, but these are too overkill in
> the specific case: there is no way "debian" could be interpreted as an
> non-trivial regex, so we can use a plain grep here.

LGTM but i cannot approve it. I'd say this one is trivial and obvious
so you could sneak it in under the "obvious" rule..
Thanks for the patch!

> 
> gcc/fortran/ChangeLog:
> 
>   * Make-lang.in: Use grep instead of fgrep.
> ---
>  gcc/fortran/Make-lang.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/fortran/Make-lang.in b/gcc/fortran/Make-lang.in
> index 1cb47cb1a52..6eb597d0ca0 100644
> --- a/gcc/fortran/Make-lang.in
> +++ b/gcc/fortran/Make-lang.in
> @@ -278,7 +278,7 @@ $(DESTDIR)$(man1dir)/$(GFORTRAN_INSTALL_NAME)$(man1ext): 
> doc/gfortran.1 \
>   -chmod a-x $@
>  
>  fortran.uninstall:
> - if $(SHELL) -c 'install-info --version | sed 1q | fgrep -s -v -i 
> debian' >/dev/null 2>&1; then \
> + if $(SHELL) -c 'install-info --version | sed 1q | grep -s -v -i debian' 
> >/dev/null 2>&1; then \
> echo " install-info --delete --info-dir=$(DESTDIR)$(infodir) 
> $(DESTDIR)$(infodir)/gfortran.info"; \
> install-info --delete --info-dir=$(DESTDIR)$(infodir) 
> $(DESTDIR)$(infodir)/gfortran.info || : ; \
>   else : ; fi; \

Re: [PATCH 1/2] Fortran: dump-parse-tree attribs: fix unbalanced braces [PR109624]

[Bernhard Reutner-Fischer via Fortran]

[re-adding the lists, i hope you don't mind]

On Wed, 10 May 2023 18:52:54 +0200
Thomas Koenig  wrote:

> Hi Bernhard,
> 
> both patches look good to me.

Pushed as r14-664-g39f7c0963a9c00 and r14-665-gbdc10c2bfaceb3
Thanks!

> 
> No user impact, so they should have the lowest possible impact :-)
> 
> (And I didn't know about DEBUG_FUNCTION, that could come in handy
> later).
> 
> Thanks for the patch!
> 
> Best regards
> 
>      Thomas

[PATCH 1/2] Fortran: dump-parse-tree attribs: fix unbalanced braces [PR109624]

[Bernhard Reutner-Fischer via Fortran]

From: Bernhard Reutner-Fischer 

gcc/fortran/ChangeLog:

PR fortran/109624
* dump-parse-tree.cc (debug): New function for gfc_namespace.
(gfc_debug_code): Delete forward declaration.
(show_attr): Make sure to print balanced braces.

---
(gdb) call debug(gfc_current_ns)

Namespace: A-H: (REAL 4) I-N: (INTEGER 4) O-Z: (REAL 4)
procedure name = fmodule
  symtree: 'C_ptr'   || symbol: 'c_ptr'
type spec : (UNKNOWN 0)
attributes: )

There is an open brace missing after "attributes: "

Regression tested on x86_64-linux, OK for trunk?
---
 gcc/fortran/dump-parse-tree.cc | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/gcc/fortran/dump-parse-tree.cc b/gcc/fortran/dump-parse-tree.cc
index 1fc1f311e84..2380fa04796 100644
--- a/gcc/fortran/dump-parse-tree.cc
+++ b/gcc/fortran/dump-parse-tree.cc
@@ -125,6 +125,16 @@ void debug (gfc_ref *p)
   dumpfile = tmp;
 }
 
+void
+debug (gfc_namespace *ns)
+{
+  FILE *tmp = dumpfile;
+  dumpfile = stderr;
+  show_namespace (ns);
+  fputc ('\n', dumpfile);
+  dumpfile = tmp;
+}
+
 void
 gfc_debug_expr (gfc_expr *e)
 {
@@ -136,7 +146,6 @@ gfc_debug_expr (gfc_expr *e)
 }
 
 /* Allow for dumping of a piece of code in the debugger.  */
-void gfc_debug_code (gfc_code *c);
 
 void
 gfc_debug_code (gfc_code *c)
@@ -758,12 +767,13 @@ show_expr (gfc_expr *p)
 static void
 show_attr (symbol_attribute *attr, const char * module)
 {
+  fputc ('(', dumpfile);
   if (attr->flavor != FL_UNKNOWN)
 {
   if (attr->flavor == FL_DERIVED && attr->pdt_template)
-   fputs (" (PDT-TEMPLATE", dumpfile);
+   fputs ("PDT-TEMPLATE ", dumpfile);
   else
-fprintf (dumpfile, "(%s ", gfc_code2string (flavors, attr->flavor));
+   fprintf (dumpfile, "%s ", gfc_code2string (flavors, attr->flavor));
 }
   if (attr->access != ACCESS_UNKNOWN)
 fprintf (dumpfile, "%s ", gfc_code2string (access_types, attr->access));
-- 
2.30.2

[PATCH 2/2] Fortran: dump-parse-tree: Mark debug functions with DEBUG_FUNCTION

[Bernhard Reutner-Fischer via Fortran]

From: Bernhard Reutner-Fischer 

gcc/fortran/ChangeLog:

* dump-parse-tree.cc (gfc_debug_expr): Remove forward declaration.
(debug): Add DEBUG_FUNCTION.
(show_code_node): Remove erroneous whitespace.

---
Regression tested on x86_64-linux, OK for trunk?
---
 gcc/fortran/dump-parse-tree.cc | 38 --
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/gcc/fortran/dump-parse-tree.cc b/gcc/fortran/dump-parse-tree.cc
index 2380fa04796..644f8f37d63 100644
--- a/gcc/fortran/dump-parse-tree.cc
+++ b/gcc/fortran/dump-parse-tree.cc
@@ -55,10 +55,8 @@ static void show_typespec (gfc_typespec *);
 static void show_ref (gfc_ref *);
 static void show_attr (symbol_attribute *, const char *);
 
-/* Allow dumping of an expression in the debugger.  */
-void gfc_debug_expr (gfc_expr *);
-
-void debug (symbol_attribute *attr)
+DEBUG_FUNCTION void
+debug (symbol_attribute *attr)
 {
   FILE *tmp = dumpfile;
   dumpfile = stderr;
@@ -67,7 +65,8 @@ void debug (symbol_attribute *attr)
   dumpfile = tmp;
 }
 
-void debug (gfc_formal_arglist *formal)
+DEBUG_FUNCTION void
+debug (gfc_formal_arglist *formal)
 {
   FILE *tmp = dumpfile;
   dumpfile = stderr;
@@ -80,12 +79,14 @@ void debug (gfc_formal_arglist *formal)
   dumpfile = tmp;
 }
 
-void debug (symbol_attribute attr)
+DEBUG_FUNCTION void
+debug (symbol_attribute attr)
 {
   debug ();
 }
 
-void debug (gfc_expr *e)
+DEBUG_FUNCTION void
+debug (gfc_expr *e)
 {
   FILE *tmp = dumpfile;
   dumpfile = stderr;
@@ -102,7 +103,8 @@ void debug (gfc_expr *e)
   dumpfile = tmp;
 }
 
-void debug (gfc_typespec *ts)
+DEBUG_FUNCTION void
+debug (gfc_typespec *ts)
 {
   FILE *tmp = dumpfile;
   dumpfile = stderr;
@@ -111,12 +113,14 @@ void debug (gfc_typespec *ts)
   dumpfile = tmp;
 }
 
-void debug (gfc_typespec ts)
+DEBUG_FUNCTION void
+debug (gfc_typespec ts)
 {
   debug ();
 }
 
-void debug (gfc_ref *p)
+DEBUG_FUNCTION void
+debug (gfc_ref *p)
 {
   FILE *tmp = dumpfile;
   dumpfile = stderr;
@@ -125,7 +129,7 @@ void debug (gfc_ref *p)
   dumpfile = tmp;
 }
 
-void
+DEBUG_FUNCTION void
 debug (gfc_namespace *ns)
 {
   FILE *tmp = dumpfile;
@@ -135,7 +139,7 @@ debug (gfc_namespace *ns)
   dumpfile = tmp;
 }
 
-void
+DEBUG_FUNCTION void
 gfc_debug_expr (gfc_expr *e)
 {
   FILE *tmp = dumpfile;
@@ -147,7 +151,7 @@ gfc_debug_expr (gfc_expr *e)
 
 /* Allow for dumping of a piece of code in the debugger.  */
 
-void
+DEBUG_FUNCTION void
 gfc_debug_code (gfc_code *c)
 {
   FILE *tmp = dumpfile;
@@ -157,7 +161,8 @@ gfc_debug_code (gfc_code *c)
   dumpfile = tmp;
 }
 
-void debug (gfc_symbol *sym)
+DEBUG_FUNCTION void
+debug (gfc_symbol *sym)
 {
   FILE *tmp = dumpfile;
   dumpfile = stderr;
@@ -2513,7 +2518,7 @@ show_code_node (int level, gfc_code *c)
 case EXEC_SYNC_MEMORY:
   fputs ("SYNC MEMORY ", dumpfile);
   if (c->expr2 != NULL)
-   {
+   {
  fputs (" stat=", dumpfile);
  show_expr (c->expr2);
}
@@ -4031,7 +4036,8 @@ gfc_dump_global_symbols (FILE *f)
 
 /* Show an array ref.  */
 
-void debug (gfc_array_ref *ar)
+DEBUG_FUNCTION void
+debug (gfc_array_ref *ar)
 {
   FILE *tmp = dumpfile;
   dumpfile = stderr;
-- 
2.30.2

[PATCH v2] Fortran: Narrow return types [PR78798]

[Bernhard Reutner-Fischer via Fortran]

From: Bernhard Reutner-Fischer 

gcc/fortran/ChangeLog:

PR fortran/78798
* array.cc (compare_bounds): Use narrower return type.
(gfc_compare_array_spec): Likewise.
(is_constant_element): Likewise.
(gfc_constant_ac): Likewise.
* check.cc (dim_rank_check): Likewise.
* cpp.cc (gfc_cpp_init_options): Likewise.
(dump_macro): Likewise.
* cpp.h (gfc_cpp_handle_option): Likewise.
* dependency.cc (gfc_ref_needs_temporary_p): Likewise.
(gfc_check_argument_dependency): Likewise.
(gfc_check_fncall_dependency): Likewise.
(ref_same_as_full_array): Likewise.
* dependency.h (gfc_check_fncall_dependency): Likewise.
(gfc_dep_resolver): Likewise.
(gfc_are_equivalenced_arrays): Likewise.
* expr.cc (gfc_copy_ref): Likewise.
(gfc_kind_max): Likewise.
(numeric_type): Likewise.
* gfortran.h (gfc_at_end): Likewise.
(gfc_at_eof): Likewise.
(gfc_at_bol): Likewise.
(gfc_at_eol): Likewise.
(gfc_define_undef_line): Likewise.
(gfc_wide_is_printable): Likewise.
(gfc_wide_is_digit): Likewise.
(gfc_wide_fits_in_byte): Likewise.
(gfc_find_sym_tree): Likewise.
(gfc_generic_intrinsic): Likewise.
(gfc_specific_intrinsic): Likewise.
(gfc_intrinsic_actual_ok): Likewise.
(gfc_has_vector_index): Likewise.
(gfc_numeric_ts): Likewise.
(gfc_impure_variable): Likewise.
(gfc_pure): Likewise.
(gfc_implicit_pure): Likewise.
(gfc_elemental): Likewise.
(gfc_pure_function): Likewise.
(gfc_implicit_pure_function): Likewise.
(gfc_compare_array_spec): Likewise.
(gfc_constant_ac): Likewise.
(gfc_expanded_ac): Likewise.
(gfc_check_digit): Likewise.
* intrinsic.cc (gfc_find_subroutine): Likewise.
(gfc_generic_intrinsic): Likewise.
(gfc_specific_intrinsic): Likewise.
* io.cc (compare_to_allowed_values): Likewise. And remove
unneeded forward declaration.
* misc.cc (gfc_done_2): Likewise.
* parse.cc: Likewise.
* parse.h (gfc_check_do_variable): Likewise.
* primary.cc (gfc_check_digit): Likewise.
* resolve.cc (resolve_structure_cons): Likewise.
(pure_stmt_function): Likewise.
(gfc_pure_function): Likewise.
(impure_stmt_fcn): Likewise.
(resolve_forall_iterators): Likewise.
(resolve_data): Likewise.
(gfc_impure_variable): Likewise.
(gfc_pure): Likewise.
(gfc_unset_implicit_pure): Likewise.
* scanner.cc (wide_is_ascii): Likewise.
(gfc_wide_toupper): Likewise.
(gfc_open_included_file): Likewise.
(gfc_at_end): Likewise.
(gfc_at_eof): Likewise.
(gfc_at_bol): Likewise.
(skip_comment_line): Likewise.
(gfc_gobble_whitespace): Likewise.
* symbol.cc (gfc_find_symtree_in_proc): Likewise.
* trans-array.cc: Likewise.
* trans-decl.cc (gfc_set_decl_assembler_name): Likewise.
* trans-types.cc (gfc_get_element_type): Likewise.
(gfc_add_field_to_struct): Likewise.
* trans-types.h (gfc_copy_dt_decls_ifequal): Likewise.
(gfc_return_by_reference): Likewise.
(gfc_is_nodesc_array): Likewise.
* trans.h (gfc_can_put_var_on_stack): Likewise.
---
Bootstrapped without new warnings and regression tested on
x86_64-linux with no regressions, OK for trunk?

 gcc/fortran/array.cc   |  8 +++
 gcc/fortran/check.cc   |  2 +-
 gcc/fortran/cpp.cc |  3 +--
 gcc/fortran/cpp.h  |  2 +-
 gcc/fortran/dependency.cc  |  8 +++
 gcc/fortran/dependency.h   |  6 ++---
 gcc/fortran/expr.cc|  6 ++---
 gcc/fortran/gfortran.h | 48 +++---
 gcc/fortran/intrinsic.cc   |  6 ++---
 gcc/fortran/io.cc  | 13 ++-
 gcc/fortran/parse.cc   |  2 +-
 gcc/fortran/parse.h|  2 +-
 gcc/fortran/primary.cc |  4 ++--
 gcc/fortran/resolve.cc | 22 -
 gcc/fortran/scanner.cc | 20 
 gcc/fortran/symbol.cc  |  2 +-
 gcc/fortran/trans-array.cc |  2 +-
 gcc/fortran/trans-decl.cc  |  2 +-
 gcc/fortran/trans-types.cc |  6 ++---
 gcc/fortran/trans-types.h  |  6 ++---
 gcc/fortran/trans.h|  2 +-
 21 files changed, 81 insertions(+), 91 deletions(-)

diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index be5eb8b6a0f..4b7c1e715bf 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -994,7 +994,7 @@ compare_bounds (gfc_expr *bound1, gfc_expr *bound2)
 /* Compares two array specifications.  They must be constant or deferred
shape.  */
 
-int
+bool
 gfc_compare_array_spec (gfc_array_spec *as1, gfc_array_spec *as2)
 {
   int i;
@@ -1039,7 +1039,7 @@ gfc_compare_array_spec (gfc_array_spec *as1, 
gfc_array_spec *as2)
use the symbol as an implied-DO iterator

Re: [PATCH v4] libgfortran: Replace mutex with rwlock

[Bernhard Reutner-Fischer via Fortran]

On Mon,  8 May 2023 17:44:43 +0800
Lipeng Zhu  wrote:

> This patch try to introduce the rwlock and split the read/write to
> unit_root tree and unit_cache with rwlock instead of the mutex to
> increase CPU efficiency. In the get_gfc_unit function, the percentage
> to step into the insert_unit function is around 30%, in most instances,
> we can get the unit in the phase of reading the unit_cache or unit_root
> tree. So split the read/write phase by rwlock would be an approach to
> make it more parallel.
> 
> BTW, the IPC metrics can gain around 9x in our test
> server with 220 cores. The benchmark we used is
> https://github.com/rwesson/NEAT

See commentary typos below.
You did not state if you regression tested the patch?
Other than that it LGTM but i cannot approve it.

> diff --git a/libgfortran/io/async.h b/libgfortran/io/async.h
> index ad226c8e856..0033cc74252 100644
> --- a/libgfortran/io/async.h
> +++ b/libgfortran/io/async.h
> @@ -210,6 +210,128 @@
>  DEBUG_PRINTF ("%s" DEBUG_RED "ACQ:" DEBUG_NORM " %-30s %78p\n", 
> aio_prefix, #mutex, mutex); \
>} while (0)
>  
> +#ifdef __GTHREAD_RWLOCK_INIT
> +#define RWLOCK_DEBUG_ADD(rwlock) do {\
> +aio_rwlock_debug *n; \
> +n = xmalloc (sizeof(aio_rwlock_debug));  \

Missing space before the open brace: sizeof (

> diff --git a/libgfortran/io/unit.c b/libgfortran/io/unit.c
> index 82664dc5f98..62f1db21d34 100644
> --- a/libgfortran/io/unit.c
> +++ b/libgfortran/io/unit.c
> @@ -33,34 +33,36 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
> If not, see
>  
>  
>  /* IO locking rules:
> -   UNIT_LOCK is a master lock, protecting UNIT_ROOT tree and UNIT_CACHE.
> +   UNIT_RWLOCK is a master lock, protecting UNIT_ROOT tree and UNIT_CACHE.
> +   And use the rwlock to spilt read and write phase to UNIT_ROOT tree
> +   and UNIT_CACHE to increase CPU efficiency.

s/spilt/split. Maybe:

Using an rwlock improves efficiency by allowing us to separate readers
and writers of both UNIT_ROOT and UNIT_CACHE.

> @@ -350,6 +356,17 @@ retry:
>if (c == 0)
>   break;
>  }
> +  /* We did not find a unit in the cache nor in the unit list, create a new
> +(locked) unit and insert into the unit list and cache.
> +Manipulating either or both the unit list and the unit cache requires to
> +hold a write-lock [for obvious reasons]:
> +1. By separating the read/write lock, it will greatly reduce the 
> contention
> +   at the read part, while write part is not always necessary or most
> +   unlikely once the unit hit in cache.

+By separating the read/write lock, we will greatly reduce the contention
+on the read part, while the write part is unlikely once the unit hits
+the cache.

> +2. We try to balance the implementation complexity and the performance
> +   gains that fit into current cases we observed by just using a
> +   pthread_rwlock. */

Let's drop 2.
thanks,

Re: [PATCH][stage1] Remove conditionals around free()

[Bernhard Reutner-Fischer via Fortran]

On Wed, 1 Mar 2023 16:07:14 -0800
Steve Kargl  wrote:

> On Wed, Mar 01, 2023 at 10:28:56PM +0100, Bernhard Reutner-Fischer via 
> Fortran wrote:
> >  libgfortran/caf/single.c |6 ++
> >  libgfortran/io/async.c   |6 ++
> >  libgfortran/io/format.c  |3 +--
> >  libgfortran/io/transfer.c|6 ++
> >  libgfortran/io/unix.c|3 +--  
> 
> The Fortran ones are OK.
> 

I've pushed the fortran and libgfortran bits as r14-568-gca2f64d5d08c16
thanks,

Re: ping Re: [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks

[Bernhard Reutner-Fischer via Fortran]

On Mon, 17 Apr 2023 15:18:27 -0700
Steve Kargl  wrote:
> On Mon, Apr 17, 2023 at 09:47:50PM +0200, Bernhard Reutner-Fischer via 
> Fortran wrote:
> > On Mon, 03 Apr 2023 23:42:06 +0200
> > Bernhard Reutner-Fischer  wrote:
> >   
> > > On 3 April 2023 21:50:49 CEST, Harald Anlauf  wrote:  
> > > >Hi Bernhard,
> > > >
> > > >there is neither context nor a related PR with a testcase showing
> > > >that this patch fixes issues seen there.
> > > 
> > > Yes, i forgot to mention the PR:
> > > 
> > > PR fortran/68800
> > > 
> > > I did not construct individual test cases but it should be obvious that 
> > > we should not leak these.
> > >   
> > > >
> > > >On 4/2/23 17:05, Bernhard Reutner-Fischer via Gcc-patches wrote:
> > > >> From: Bernhard Reutner-Fischer 
> > > >> 
> > > >> Cc: fortran@gcc.gnu.org
> > > >> 
> > > >> gcc/fortran/ChangeLog:
> > > >> 
> > > >>* array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing.
> > > >>* expr.cc (find_array_section): Fix mpz memory leak.
> > > >>* simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in
> > > >>error paths.
> > > >>(gfc_simplify_set_exponent): Fix mpfr memory leak.
> > > >> ---
> > > >>   gcc/fortran/array.cc| 3 +++
> > > >>   gcc/fortran/expr.cc | 8 
> > > >>   gcc/fortran/simplify.cc | 7 ++-
> > > >>   3 files changed, 13 insertions(+), 5 deletions(-)
> > > >> 
> > > >> diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
> > > >> index be5eb8b6a0f..8b1e816a859 100644
> > > >> --- a/gcc/fortran/array.cc
> > > >> +++ b/gcc/fortran/array.cc
> > > >> @@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int 
> > > >> dimen, mpz_t *result, mpz_t *end)
> > > >> return t;
> > > >> 
> > > >>   default:
> > > >> +  mpz_clear (lower);
> > > >> +  mpz_clear (stride);
> > > >> +  mpz_clear (upper);
> > > >> gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type");
> > > >>   }
> > > >
> > > >  What is the point of clearing variables before issuing
> > > >  a gfc_internal_error?
> > > 
> > > To make it obvious that we are aware that we allocated these.  
> 
> I must admit I agree with Harald on this portion
> of the diff.  What's the point?  There is alot more
> allocated than just those 3 mzp_t variables when the
> internal error occurs.  For example, gfortran does not
> walk the namespaces and free those before the ICE.
> I suppose silencing valgrind might be sufficient 
> reason for the clutter.  So, ok.

I've dropped this hunk and pushed the rest as r14-567-g2521390dd2f8e5
Harald fixed the leak of expr in gfc_simplify_set_exponent in the
meantime.
thanks,

Re: [committed] Fortran: Fix (mostly) comment typos

[Bernhard Reutner-Fischer via Fortran]

On 28 April 2023 09:26:06 CEST, Tobias Burnus  wrote:
>Committed as r14-319-g7ebd4a1d61993c0a75e9ff3098aded21ef04a4da

 >  Only other changes are fixing the variable name a(b)breviated_modproc_decl

I think this is not good, I've mentioned it somewhere, i think, but I'll rename 
it.
thanks!

Re: libsanitizer: sync from master

[Bernhard Reutner-Fischer via Fortran]

On 28 April 2023 11:23:55 CEST, Florian Weimer via Fortran 
 wrote:
>* Martin Liška:

>But that's okay for me as well.

Even better.

Re: libsanitizer: sync from master

[Bernhard Reutner-Fischer via Fortran]

On 26 April 2023 20:31:10 CEST, Florian Weimer via Fortran 
 wrote:
>* Martin Liška:
>
>> On 11/15/22 16:47, Martin Liška wrote:
>>> Hi.
>>> 
>>> I've just pushed libsanitizer update that was tested on x86_64-linux and 
>>> ppc64le-linux systems.
>>> Moreover, I run bootstrap on x86_64-linux and checked ABI difference with 
>>> abidiff.
>>
>> Hello.
>>
>> And I've done the same now and merged upstream version 3185e47b5a8444e9fd.
>
>So … we have the issue that involves interceptors outside of libc.so.6,
>namely crypt, crypt_r, and I posted an upstream patch for this:
>
>  sanitizers: Disable crypt, crypt_r interceptors for glibc
>  
>
>Can we just apply this downstream for now?  It blocks various folks from
>using the sanitizers in their projects.

+1

Re: [PATCH v3] libgfortran: Replace mutex with rwlock

[Bernhard Reutner-Fischer via Fortran]

Hi!

[please do not top-post]

On Thu, 20 Apr 2023 21:13:08 +0800
"Zhu, Lipeng"  wrote:

> Hi Bernhard,
> 
> Thanks for your questions and suggestions.
> The rwlock could allow multiple threads to have concurrent read-only 
> access to the cache/unit list, only a single writer is allowed.

right.

> Write lock will not be acquired until all read lock are released.

So i must have confused rwlock with something else, something that
allows self to hold a read-lock and upgrade that to a write-lock,
purposely starving all successive incoming readers. I.e. just toggle
your RD_TO_WRLOCK impl, here, atomically. This proved to be benefical in
some situations in the past. Doesn't seem to work with your rwlock,
does it

> And I didn't change the mutex scope when refactor the code, only make a 
> more fine-grained distinction for the read/write cache/unit list.

Yes of course, i can see you did that.

> I complete the comment according to your template, I will insert the 
> comment in the source code in next version patch with other refinement 
> by your suggestions.
> "
> Now we did not get a unit in cache and unit list, so we need to create a
> new unit, and update it to cache and unit list.

s/Now/By now/ or s/Now w/W/ and s/get/find/
"
We did not find a unit in the cache nor in the unit list, create a new
(locked) unit and insert into the unit list and cache.
Manipulating either or both the unit list and the unit cache requires to
hold a write-lock [for obvious reasons]"

Superfluous when talking about pthread_rwlock_wrlock since that
implies that even the process acquiring the wrlock has to first
release it's very own rdlock.

> Prior to update the cache and list, we need to release all read locks,
> and then immediately to acquire write lock, thus ensure the exclusive
> update to the cache and unit list.
> Either way, we will manipulate the cache and/or the unit list so we must
> take a write lock now.
> We don't take the write bit in *addition* to the read lock because:
> 1. It will needlessly complicate releasing the respective lock;

Under pthread_rwlock_wrlock it will deadlock, so that's wrong?
Drop that point then? If not, what's your reasoning / observation?

Under my lock, you hold the R, additionally take the W and then
immediately release the R because you yourself won't read, just write.
But mine's not the pthread_rwlock you talk about, admittedly.

> 2. By separate the read/write lock, it will greatly reduce the
> contention at the read part, while write part is not always necessary or
> most unlikely once the unit hit in cache;

We know that.

> 3. We try to balance the implementation complexity and the performance
> gains that fit into current cases we observed.

.. by just using a pthread_rwlock. And that's the top point iff you
keep it at that. That's a fair step, sure. BTW, did you look at the
RELEASE semantics, respectively the note that one day (and now is that
very day), we might improve on the release semantics? Can of course be
incremental AFAIC

> "

If folks agree on this first step then you have my OK with a catchy
malloc and the discussion recorded here on the list. A second step would
be RELEASE.
And, depending on the underlying capabilities of available locks,
further tweaks, obviously.

PS: and, please, don't top-post

thanks,

> 
> Best Regards,
> Zhu, Lipeng
> 
> On 1/1/1970 8:00 AM, Bernhard Reutner-Fischer wrote:
> > On 19 April 2023 09:06:28 CEST, Lipeng Zhu via Fortran 
> >  wrote:  
> >> This patch try to introduce the rwlock and split the read/write to
> >> unit_root tree and unit_cache with rwlock instead of the mutex to
> >> increase CPU efficiency. In the get_gfc_unit function, the percentage
> >> to step into the insert_unit function is around 30%, in most instances,
> >> we can get the unit in the phase of reading the unit_cache or unit_root
> >> tree. So split the read/write phase by rwlock would be an approach to
> >> make it more parallel.
> >>
> >> BTW, the IPC metrics can gain around 9x in our test server with 220
> >> cores. The benchmark we used is https://github.com/rwesson/NEAT
> >>  
> >   
> >> +#define RD_TO_WRLOCK(rwlock) \
> >> +  RWUNLOCK (rwlock);\
> >> +  WRLOCK (rwlock);
> >> +#endif
> >> +  
> > 
> >   
> >> diff --git a/libgfortran/io/unit.c b/libgfortran/io/unit.c index
> >> 82664dc5f98..4312c5f36de 100644
> >> --- a/libgfortran/io/unit.c
> >> +++ b/libgfortran/io/unit.c  
> >   
> >> @@ -329,7 +335,7 @@ get_gfc_unit (int n, int do_create)
> >>int c, created = 0;
> >>
> >>NOTE ("Unit n=%d, do_create = %d", n, do_create);
> >> -  LOCK (_lock);
> &

Re: (GCC) 13.0.1: internal compiler error

[Bernhard Reutner-Fischer via Fortran]

Cc:ing Thomas, who knows openacc better.

There is a devel/omp/gcc-12 branch you might want to try. I don't know
how different that branch is wrt openacc.

HTH,

On Mon, 24 Apr 2023 19:39:15
+0200 Patrick Begou via Fortran  wrote:

> Hi Harald
> 
> as I said, it is a large, massively parallel fortran code: more than 700 
> files, some with several thousands of lines. It could be difficult to 
> create a small reproducer but I will try if the problem is not known as 
> an other git branch of this code also create an internal error on 
> another file.
> 
> Best regards
> 
> Patrick
> 
> Le 24/04/2023 à 19:27, Harald Anlauf a écrit :
> > Hi Patrick,
> >
> > I did not see any similar report in bugzilla, so could you please
> > open a PR and attach a self-contained reproducer?  Ideally the
> > reproducer would be reduced to simplify the analysis for those
> > familiar with the status of the OpenACC implementation.
> >
> > Thanks,
> > Harald
> >
> > Am 21.04.23 um 17:13 schrieb Patrick Begou:  
> >> Hi,
> >>
> >> I have built this morning the latest gfortran from a git clone:
> >>
> >> GNU Fortran (GCC) 13.0.1 20230421 (prerelease)
> >>
> >> I'm trying this compiler on a large and complexe Fortran90 code with
> >> offloading using OpenACC.
> >>
> >> At this time:
> >>
> >> - code compiles with nvfortran and runs on A100 GPU.
> >>
> >> - code compiles with Cray Fortran (with some difficulties) but do not
> >> run on MI250 GPU (we are tacking the problem, a segfault if openacc is
> >> set on)
> >>
> >> - code compile with GNU GCC 13 without -fopenacc option and runs on cpu
> >> (Epyc2 7302)
> >>
> >> - a basic test-code using OpenACC compiles and run on the GPU.
> >>
> >> - compiling my large code with gcc 13.0.1 using -fopenacc for A100 GPU
> >> produce an internal error in the compiler :
> >>
> >>
> >> transforms_defs_m.f90:354:53:
> >>
> >>    354 | !$acc enter data attach(atransform2%next)
> >>    | ^
> >> internal compiler error: in omp_group_base, at gimplify.cc:9412
> >> 0xa830c6 omp_group_base
> >>      ../../gcc/gcc/gimplify.cc:9412
> >> 0xa830c6 omp_index_mapping_groups_1
> >>      ../../gcc/gcc/gimplify.cc:9441
> >> 0xa833c7 omp_index_mapping_groups
> >>      ../../gcc/gcc/gimplify.cc:9502
> >> 0xa96a9a gimplify_scan_omp_clauses
> >>      ../../gcc/gcc/gimplify.cc:10802
> >> 0xa8660d gimplify_omp_target_update
> >>      ../../gcc/gcc/gimplify.cc:15563
> >> 0xa8660d gimplify_expr(tree_node**, gimple**, gimple**, bool
> >> (*)(tree_node*), int)
> >>      ../../gcc/gcc/gimplify.cc:16928
> >> 0xa89826 gimplify_stmt(tree_node**, gimple**)
> >>      ../../gcc/gcc/gimplify.cc:7219
> >> 0xa875a3 gimplify_statement_list
> >>      ../../gcc/gcc/gimplify.cc:2019
> >> 0xa875a3 gimplify_expr(tree_node**, gimple**, gimple**, bool
> >> (*)(tree_node*), int)
> >>      ../../gcc/gcc/gimplify.cc:16821
> >> 0xa89826 gimplify_stmt(tree_node**, gimple**)
> >>      ../../gcc/gcc/gimplify.cc:7219
> >> 0xa86e8a gimplify_and_add(tree_node*, gimple**)
> >>      ../../gcc/gcc/gimplify.cc:492
> >> 0xa86e8a gimplify_loop_expr
> >>      ../../gcc/gcc/gimplify.cc:1993
> >> 0xa86e8a gimplify_expr(tree_node**, gimple**, gimple**, bool
> >> (*)(tree_node*), int)
> >>      ../../gcc/gcc/gimplify.cc:16581
> >> 0xa89826 gimplify_stmt(tree_node**, gimple**)
> >>      ../../gcc/gcc/gimplify.cc:7219
> >> 0xa875a3 gimplify_statement_list
> >>      ../../gcc/gcc/gimplify.cc:2019
> >> 0xa875a3 gimplify_expr(tree_node**, gimple**, gimple**, bool
> >> (*)(tree_node*), int)
> >>      ../../gcc/gcc/gimplify.cc:16821
> >> 0xa89826 gimplify_stmt(tree_node**, gimple**)
> >>      ../../gcc/gcc/gimplify.cc:7219
> >> 0xa89d2b gimplify_bind_expr
> >>      ../../gcc/gcc/gimplify.cc:1430
> >> 0xa86d8e gimplify_expr(tree_node**, gimple**, gimple**, bool
> >> (*)(tree_node*), int)
> >>      ../../gcc/gcc/gimplify.cc:16577
> >> 0xa89826 gimplify_stmt(tree_node**, gimple**)
> >>      ../../gcc/gcc/gimplify.cc:7219
> >> Please submit a full bug report, with preprocessed source (by using
> >> -freport-bug).
> >>
> >>
> >> Options used (I've just added  -fopenacc for moving from cpu version to
> >> OpenACC):
> >>
> >> -fopenacc -freport-bug -g -fpic -x f95-cpp-input -std=gnu -ffree-form
> >> -fall-intrinsics -fallow-argument-mismatch -Wall -Wextra -W
> >> -Wno-unused-function -Wno-compare-reals -fno-omit-frame-pointer -O3
> >> -ftree-vectorize -ffast-math -funroll-loops -pipe
> >>
> >> No additionnal files a produced with -freport-bug.
> >>
> >> In attachment the script used to build the compiler.
> >>
> >> Let me know how I can help with informations to improve Gnu fortran
> >> compilers.
> >>
> >> Patrick  
> >  
> 

Re: [Patch, fortran] PRs 105152, 100193, 87946, 103389, 104429 and 82774

[Bernhard Reutner-Fischer via Fortran]

On Sun, 23 Apr 2023 23:48:03 +0200
Harald Anlauf via Fortran  wrote:

> Am 22.04.23 um 10:32 schrieb Paul Richard Thomas via Gcc-patches:

> > PR103931 - I couldn't reproduce the bug, which involves 'ambiguous c_ptr'.
> > To judge by the comments, it seems that this bug is a bit elusive.

See Haralds comment 12, you need to remove the use cmodule:
module DModule
   use AModule
   !comment 12, 'use CModule' should not be needed: use CModule
   !use CModule

   implicit none
   private

   public :: DType

   type, abstract :: DType
   end type
end module

> PR103931: it is indeed a bit elusive, but very sensitive to code
> changes.  Also Bernhard had a look at it.  Given that there are
> a couple of bugs related to module reading, and rename-on-use,
> I'd recommend to leave that open for further analysis.

I would mark the dt sym that is used *as* the generic interface with
attr.generic.

Like: https://gcc.gnu.org/PR103931#c18

This seems to work and sounds somewhat plausible (to me).
If that is not correct, then i'm running out of ideas and will stop
looking at that PR.

cheers,

Re: [PATCH v3] libgfortran: Replace mutex with rwlock

[Bernhard Reutner-Fischer via Fortran]

On 19 April 2023 09:06:28 CEST, Lipeng Zhu via Fortran  
wrote:
>This patch try to introduce the rwlock and split the read/write to
>unit_root tree and unit_cache with rwlock instead of the mutex to
>increase CPU efficiency. In the get_gfc_unit function, the percentage
>to step into the insert_unit function is around 30%, in most instances,
>we can get the unit in the phase of reading the unit_cache or unit_root
>tree. So split the read/write phase by rwlock would be an approach to
>make it more parallel.
>
>BTW, the IPC metrics can gain around 9x in our test
>server with 220 cores. The benchmark we used is
>https://github.com/rwesson/NEAT
>

>+#define RD_TO_WRLOCK(rwlock) \
>+  RWUNLOCK (rwlock);\
>+  WRLOCK (rwlock);
>+#endif
>+


>diff --git a/libgfortran/io/unit.c b/libgfortran/io/unit.c
>index 82664dc5f98..4312c5f36de 100644
>--- a/libgfortran/io/unit.c
>+++ b/libgfortran/io/unit.c

>@@ -329,7 +335,7 @@ get_gfc_unit (int n, int do_create)
>   int c, created = 0;
> 
>   NOTE ("Unit n=%d, do_create = %d", n, do_create);
>-  LOCK (_lock);
>+  RDLOCK (_rwlock);
> 
> retry:
>   for (c = 0; c < CACHE_SIZE; c++)
>@@ -350,6 +356,7 @@ retry:
>   if (c == 0)
>   break;
> }
>+  RD_TO_WRLOCK (_rwlock);

So I'm trying to convince myself why it's safe to unlock and only then take the 
write lock.

Can you please elaborate/confirm why that's ok?

I wouldn't mind a comment like
We can release the unit and cache read lock now. We might have to allocate a 
(locked) unit, below in do_create.
Either way, we will manipulate the cache and/or the unit list so we have to 
take a write lock now.

We don't take the write bit in *addition* to the read lock because..

(that needlessly complicates releasing the respective locks / it triggers too 
much contention when we.. / ...?)

thanks,

> 
>   if (p == NULL && do_create)
> {
>@@ -368,8 +375,8 @@ retry:
>   if (created)
> {
>   /* Newly created units have their lock held already
>-   from insert_unit.  Just unlock UNIT_LOCK and return.  */
>-  UNLOCK (_lock);
>+   from insert_unit.  Just unlock UNIT_RWLOCK and return.  */
>+  RWUNLOCK (_rwlock);
>   return p;
> }
> 
>@@ -380,7 +387,7 @@ found:
>   if (! TRYLOCK (>lock))
>   {
> /* assert (p->closed == 0); */
>-UNLOCK (_lock);
>+RWUNLOCK (_rwlock);
> return p;
>   }
> 
>@@ -388,14 +395,14 @@ found:
> }
> 
> 
>-  UNLOCK (_lock);
>+  RWUNLOCK (_rwlock);
> 
>   if (p != NULL && (p->child_dtio == 0))
> {
>   LOCK (>lock);
>   if (p->closed)
>   {
>-LOCK (_lock);
>+WRLOCK (_rwlock);
> UNLOCK (>lock);
> if (predec_waiting_locked (p) == 0)
>   destroy_unit_mutex (p);
>@@ -593,8 +600,8 @@ init_units (void)
> #endif
> #endif
> 
>-#ifndef __GTHREAD_MUTEX_INIT
>-  __GTHREAD_MUTEX_INIT_FUNCTION (_lock);
>+#if (!defined(__GTHREAD_RWLOCK_INIT) && !defined(__GTHREAD_MUTEX_INIT))
>+  __GTHREAD_MUTEX_INIT_FUNCTION (_rwlock);
> #endif
> 
>   if (sizeof (max_offset) == 8)

Re: [PATCH] testsuite: fix scan-tree-dump patterns [PR83904, PR100297]

[Bernhard Reutner-Fischer via Fortran]

On Wed, 19 Apr 2023 at 03:03, Jerry D via Fortran  wrote:
>
> On 4/18/23 12:39 PM, Harald Anlauf via Fortran wrote:
> > Dear all,
> >
> > the attached patch adjusts the scan-tree-dump patterns of the
> > reported testcases which likely were run in a location such
> > that a path in an error message showing in the tree-dump might
> > have accidentally matched "free" or "data", respectively.
> >
> > For the testcase gfortran.dg/reshape_8.f90 I checked with a
> > failing gfortran-11 that the pattern is appropriate.
> >
> > OK for mainline?
> >
> > Thanks,
> > Harald
> >
> Yes, OK

I'm certainly not opposed to this specific incarnation of such a fix.
These failures are really unpleasant :)
As proposed in 
https://inbox.sourceware.org/gcc-patches/20220426010029.2b476337@nbbrfq/
we could add a -fno-file to suppress the assembler .file output
(whatever the prefix looks like depends on the assembler dialect). Or
we could nuke the .file directives by a sed(1), but that would
probably be cumbersome for remote targets. I don't have a better idea
than -fno-file or -ffile=foo.c .
Fixing them case-by-case does not scale all that well IMHO.

Thoughts?

Re: [PATCH v3] libgfortran: Replace mutex with rwlock

[Bernhard Reutner-Fischer via Fortran]

On Wed, 19 Apr 2023 at 14:51, Bernhard Reutner-Fischer
 wrote:
>
> On 19 April 2023 09:06:28 CEST, Lipeng Zhu via Fortran  
> wrote:
>
> >+#ifdef __GTHREAD_RWLOCK_INIT
> >+#define RWLOCK_DEBUG_ADD(rwlock) do { \
> >+aio_rwlock_debug *n;  \
> >+n = malloc (sizeof(aio_rwlock_debug));\
>
> My malloc can fail.

Sorry, i hit send too early.
Please use xmalloc as defined in libgfortran/runtime/memory.c

PS: IIRC we have likely() / unlikely() macros in libgfortran, so you
may want to check if you want to annotate some conditions accordingly
if predict gets it wrong.
thanks,
>
> >+n->prev = TAIL_RWLOCK_DEBUG_QUEUE;\
> >+if (n->prev)  \
> >+  n->prev->next = n;  \
> >+n->next = NULL;   \
> >+n->line = __LINE__;   \
> >+n->func = __FUNCTION__;   \
> >+n->rw = rwlock;   \
> >+if (!aio_rwlock_debug_head) { \
> >+  aio_rwlock_debug_head = n;  \
> >+} \
> >+  } while (0)
> >+
>

Re: [PATCH v3] libgfortran: Replace mutex with rwlock

[Bernhard Reutner-Fischer via Fortran]

On 19 April 2023 09:06:28 CEST, Lipeng Zhu via Fortran  
wrote:

>+#ifdef __GTHREAD_RWLOCK_INIT
>+#define RWLOCK_DEBUG_ADD(rwlock) do { \
>+aio_rwlock_debug *n;  \
>+n = malloc (sizeof(aio_rwlock_debug));\

My malloc can fail.

>+n->prev = TAIL_RWLOCK_DEBUG_QUEUE;\
>+if (n->prev)  \
>+  n->prev->next = n;  \
>+n->next = NULL;   \
>+n->line = __LINE__;   \
>+n->func = __FUNCTION__;   \
>+n->rw = rwlock;   \
>+if (!aio_rwlock_debug_head) { \
>+  aio_rwlock_debug_head = n;  \
>+} \
>+  } while (0)
>+

ping Re: [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks

[Bernhard Reutner-Fischer via Fortran]

Ping!

Harald fixed the leak in set_exponent in the meantime.
As stated in the cover-letter, it was bootstrapped and regtested
without regression on x86_64-foo-linux.

I consider it obvious, but never the less, OK for trunk (as in gcc-14)
so far?

thanks,

On Mon, 03 Apr 2023 23:42:06 +0200
Bernhard Reutner-Fischer  wrote:

> On 3 April 2023 21:50:49 CEST, Harald Anlauf  wrote:
> >Hi Bernhard,
> >
> >there is neither context nor a related PR with a testcase showing
> >that this patch fixes issues seen there.  
> 
> Yes, i forgot to mention the PR:
> 
> PR fortran/68800
> 
> I did not construct individual test cases but it should be obvious that we 
> should not leak these.
> 
> >
> >On 4/2/23 17:05, Bernhard Reutner-Fischer via Gcc-patches wrote:  
> >> From: Bernhard Reutner-Fischer 
> >> 
> >> Cc: fortran@gcc.gnu.org
> >> 
> >> gcc/fortran/ChangeLog:
> >> 
> >>* array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing.
> >>* expr.cc (find_array_section): Fix mpz memory leak.
> >>* simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in
> >>error paths.
> >>(gfc_simplify_set_exponent): Fix mpfr memory leak.
> >> ---
> >>   gcc/fortran/array.cc| 3 +++
> >>   gcc/fortran/expr.cc | 8 
> >>   gcc/fortran/simplify.cc | 7 ++-
> >>   3 files changed, 13 insertions(+), 5 deletions(-)
> >> 
> >> diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
> >> index be5eb8b6a0f..8b1e816a859 100644
> >> --- a/gcc/fortran/array.cc
> >> +++ b/gcc/fortran/array.cc
> >> @@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, 
> >> mpz_t *result, mpz_t *end)
> >> return t;
> >> 
> >>   default:
> >> +  mpz_clear (lower);
> >> +  mpz_clear (stride);
> >> +  mpz_clear (upper);
> >> gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type");
> >>   }  
> >
> >What is the point of clearing variables before issuing a gfc_internal_error? 
> > 
> 
> To make it obvious that we are aware that we allocated these.
> 
> thanks,
> >  
> >> diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
> >> index 7fb33f81788..b4736804eda 100644
> >> --- a/gcc/fortran/expr.cc
> >> +++ b/gcc/fortran/expr.cc
> >> @@ -1539,6 +1539,7 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
> >> mpz_init_set_ui (delta_mpz, one);
> >> mpz_init_set_ui (nelts, one);
> >> mpz_init (tmp_mpz);
> >> +  mpz_init (ptr);
> >> 
> >> /* Do the initialization now, so that we can cleanup without
> >>keeping track of where we were.  */
> >> @@ -1682,7 +1683,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
> >> mpz_mul (delta_mpz, delta_mpz, tmp_mpz);
> >>   }
> >> 
> >> -  mpz_init (ptr);
> >> cons = gfc_constructor_first (base);
> >> 
> >> /* Now clock through the array reference, calculating the index in
> >> @@ -1735,7 +1735,8 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
> >> "at %L requires an increase of the allowed %d "
> >> "upper limit.  See %<-fmax-array-constructor%> "
> >> "option", >where, flag_max_array_constructor);
> >> -return false;
> >> +t = false;
> >> +goto cleanup;
> >>}
> >> 
> >> cons = gfc_constructor_lookup (base, limit);
> >> @@ -1750,8 +1751,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
> >>   gfc_copy_expr (cons->expr), NULL);
> >>   }
> >> 
> >> -  mpz_clear (ptr);
> >> -
> >>   cleanup:
> >> 
> >> mpz_clear (delta_mpz);
> >> @@ -1765,6 +1764,7 @@ cleanup:
> >> mpz_clear (ctr[d]);
> >> mpz_clear (stride[d]);
> >>   }
> >> +  mpz_clear (ptr);
> >> gfc_constructor_free (base);
> >> return t;
> >>   }
> >> diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc
> >> index ecf0e3558df..d1f06335e79 100644
> >> --- a/gcc/fortran/simplify.cc
> >> +++ b/gcc/fortran/simplify.cc
> >> @@ -6866,6 +6866,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
> >> *shape_exp,
> >>  gfc_error ("The SHAPE array 

Fw: GCC 14.0.0 Status Report (2023-04-17)

[Bernhard Reutner-Fischer via Fortran]

fyi

Begin forwarded message:

Date: Mon, 17 Apr 2023 15:56:29 +0200
From: Jakub Jelinek via Gcc-patches 
To: g...@gcc.gnu.org, gcc-patc...@gcc.gnu.org
Subject: GCC 14.0.0 Status Report (2023-04-17)


Status
==

The trunk has branched for the GCC 13 release and is now open
again for general development, stage 1.  Please consider not
disrupting it too much during the RC phase of GCC 13 so it
is possible to test important fixes for 13.1 on it.


Quality Data


Priority  #   Change from last report
---   ---
P10   -  32
P2  494   +   2
P3   57   -  20
P4  241   -  14
P5   24
---   ---
Total P1-P3 551   -  50
Total   816   -  64


Previous Report
===

https://gcc.gnu.org/pipermail/gcc/2023-February/240765.html

Fw: GCC 13.0.1 Status Report (2023-04-17)

[Bernhard Reutner-Fischer via Fortran]

fyi

Begin forwarded message:

Date: Mon, 17 Apr 2023 15:54:45 +0200
From: Jakub Jelinek via Gcc-patches 
To: g...@gcc.gnu.org, gcc-patc...@gcc.gnu.org
Subject: GCC 13.0.1 Status Report (2023-04-17)


Status
==

We have reached zero P1 regressions today and releases/gcc-13 branch has
been created.  GCC 13.1-rc1 will be built likely tomorrow.
The branch is now frozen for blocking regressions and documentation
fixes only, all changes to the branch require a RM approval now.

If no show stoppers appear, we'd like to release 13.1 next week,
or soon after that, if any important issues are discovered and
fixed, rc2 could be released next week.


Quality Data


Priority  #   Change from last report
---   ---
P10   -  32
P2  492
P3   57   -  20
P4  241   -  14
P5   24
---   ---
Total P1-P3 549   -  52
Total   814   -  66


Previous Report
===

https://gcc.gnu.org/pipermail/gcc/2023-February/240765.html

Re: [PATCH,FORTRAN 00/29] Move towards stringpool, part 1

[Bernhard Reutner-Fischer via Fortran]

Hi all, Janne!

On Wed, 19 Sep 2018 16:40:01 +0200
Bernhard Reutner-Fischer  wrote:
> On Fri, 7 Sep 2018 at 10:07, Bernhard Reutner-Fischer
>  wrote:
> >
> > On Wed, 5 Sep 2018 at 20:57, Janne Blomqvist  
> > wrote:  
> > >
> > > On Wed, Sep 5, 2018 at 5:58 PM Bernhard Reutner-Fischer 
> > >  wrote:  
> >  
> > >> Bootstrapped and regtested on x86_64-foo-linux.
> > >>
> > >> I'd appreciate if someone could double check for regressions on other
> > >> setups. Git branch:
> > >> https://gcc.gnu.org/git/?p=gcc.git;a=log;h=refs/heads/aldot/fortran-fe-stringpool
> > >>
> > >> Ok for trunk?  
> > >
> > >
> > > Hi,
> > >
> > > this is quite an impressive patch set. I have looked through all the 
> > > patches, and on the surface they all look ok.  
> >
> > Thanks alot for your appreciation!  
> > >
> > > Unfortunately I don't have any exotic target to test on either, so I 
> > > think you just have to commit it and check for regression reports. Though 
> > > I don't see this set doing anything which would work differently on other 
> > > targets, but you never know..
> > >
> > > I'd say wait a few days in case anybody else wants to comment on it, then 
> > > commit it to trunk.  

Unfortunately I never got to commit it.

Are you still OK with this series?

Iff so, i will refresh it for gcc-14 stage1. I would formally resubmit
the series for approval then, of course, for good measure.

It will need some small adjustments since coarrays were added
afterwards and a few other spots were changed since then.

Note that after your OK i fixed the problem described below with this
patch
https://inbox.sourceware.org/gcc-patches/20180919225533.20009-1-rep.dot@gmail.com/
and so i think this "[PATCH,FORTRAN v2] Use stringpool on loading
module symbols" was not formally OKed yet, FWIW. I believe that this v2 worked 
flawlessly.
Note, however, that by adding/changing module names of symbols in the
module files, this will (i think / fear) require a bump of the module
version if we are honest. Ultimately that was the reason why i did not
push the series back then.

Link to the old series:
https://inbox.sourceware.org/gcc-patches/20180905145732.404-1-rep.dot@gmail.com/

thanks and cheers,

> >
> > Upon further testing i encountered a regression in module writing,
> > manifesting itself in a failure to compile ieee_8.f90 (and only this).  
> 
> > Sorry for that, I'll have another look during the weekend.  
> 
> so in free_pi_tree we should not free true_name nor module:
> 
> @@ -239,12 +239,6 @@ free_pi_tree (pointer_info *p)
>free_pi_tree (p->left);
>free_pi_tree (p->right);
> 
> -  if (iomode == IO_INPUT)
> -{
> -  XDELETEVEC (p->u.rsym.true_name);
> -  XDELETEVEC (p->u.rsym.module);
> -}
> -
>free (p);
>  }
> 
> This fixes the module writing but leaks, obviously.
> Now the reason why i initially did not use mio_pool_string for both
> rsym.module and rsym.true_name was that mio_pool_string sets the name
> to NULL if the string is empty.
> Currently these are read by read_string() [which we should get rid of
> entirely, the 2 mentioned fields are the last two who use
> read_string()] which does not nullify the empty string but returns
> just the pointer. For e.g. ieee_8.f90 using mio_pool_string gives us a
> NULL module which leads to gfc_use_module -> load_needed ->
> gfc_find_symbol -> gfc_find_sym_tree -> gfc_find_symtree which tries
> to c = strcmp (name, st->name); where name is NULL.
> 
> The main culprits seem to be class finalization wrapper variables so
> i'm adding modules to those now.
> Which leaves me with regressions like allocate_with_source_14.f03.
> "Fixing" these by falling back to gfc_current_ns->proc_name->name in
> load_needed for !ns->proc_name if the rsym->module is NULL seems to
> work.
> 
> Now there are a number of issues with names of fixups. Like the 9 in e.g.:
> 
> $ zcat /tmp/n/m.mod | egrep -v "^(\(\)|\(\(\)|$)"
> GFORTRAN module version '15' created from generic_27.f90
> (('testif' 'm' 2 3))
> (4 'm' 'm' '' 1 ((MODULE UNKNOWN-INTENT UNKNOWN-PROC UNKNOWN UNKNOWN 0 0)
> 3 'test1' 'm' '' 1 ((PROCEDURE UNKNOWN-INTENT MODULE-PROC DECL UNKNOWN 0
> 0 FUNCTION) () (REAL 4 0 0 0 REAL ()) 5 0 (6) () 3 () () () 0 0)
> 2 'test2' 'm' '' 1 ((PROCEDURE UNKNOWN-INTENT MODULE-PROC DECL UNKNOWN 0
> 0 FUNCTION ARRAY_OUTER_DEPENDENCY) () (REAL 4 0 0 0 REAL ()) 7 0 (8) ()
> 2 () () () 0 0)
> 6 'obj' '' '' 5 ((VARIABLE UNKNOWN-INTENT UNKNOWN-PROC UNKNOWN UNKNOWN 0
> 0

Re: [PATCH] Introduce hardbool attribute for C

[Bernhard Reutner-Fischer via Fortran]

On 6 April 2023 08:11:11 CEST, Alexandre Oliva  wrote:
>On Apr  2, 2023, Bernhard Reutner-Fischer  wrote:
>
>> On Tue, 09 Aug 2022 10:53:08 -0300
>> Alexandre Oliva via Gcc-patches  wrote:
>
>>> Ping? (sorry, Joseph, I failed to Cc: you last time)
>
>> Didn't move yet did it.
>
>'fraid not, and surely it's too late for it to make gcc-13.
>

In fortran we have the following per the language:

---8<---
5 18.2.2 Named constants and derived types in the module
26 1 The entities listed in the second column of Table 18.2 shall be default 
integer named constants.
27 2 A Fortran intrinsic type whose kind type parameter is one of the values in 
the module shall have the same
28 representation as the C type with which it interoperates, for each value 
that a variable of that type can have.
29 For C_BOOL, the internal representation of .TRUE._C_BOOL and .FALSE._C_BOOL 
shall be the same as those of
30 the C values (_Bool)1 and (_Bool)0 respectively.

[]

12 5 The value of C_BOOL shall be a valid value for a logical kind parameter on 
the processor or shall be −1.
---8<---
thus we should probably be careful as i guess this might not work interoperable 
out of the box, fwiw. Not sure how realistic such a usecase would be..

I personally like your proposed 0 and ~0, that's probably pretty robust.
thanks,

Re: [PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks

[Bernhard Reutner-Fischer via Fortran]

On 3 April 2023 21:50:49 CEST, Harald Anlauf  wrote:
>Hi Bernhard,
>
>there is neither context nor a related PR with a testcase showing
>that this patch fixes issues seen there.

Yes, i forgot to mention the PR:

PR fortran/68800

I did not construct individual test cases but it should be obvious that we 
should not leak these.

>
>On 4/2/23 17:05, Bernhard Reutner-Fischer via Gcc-patches wrote:
>> From: Bernhard Reutner-Fischer 
>> 
>> Cc: fortran@gcc.gnu.org
>> 
>> gcc/fortran/ChangeLog:
>> 
>>  * array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing.
>>  * expr.cc (find_array_section): Fix mpz memory leak.
>>  * simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in
>>  error paths.
>>  (gfc_simplify_set_exponent): Fix mpfr memory leak.
>> ---
>>   gcc/fortran/array.cc| 3 +++
>>   gcc/fortran/expr.cc | 8 
>>   gcc/fortran/simplify.cc | 7 ++-
>>   3 files changed, 13 insertions(+), 5 deletions(-)
>> 
>> diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
>> index be5eb8b6a0f..8b1e816a859 100644
>> --- a/gcc/fortran/array.cc
>> +++ b/gcc/fortran/array.cc
>> @@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, 
>> mpz_t *result, mpz_t *end)
>> return t;
>> 
>>   default:
>> +  mpz_clear (lower);
>> +  mpz_clear (stride);
>> +  mpz_clear (upper);
>> gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type");
>>   }
>
>What is the point of clearing variables before issuing a gfc_internal_error?

To make it obvious that we are aware that we allocated these.

thanks,
>
>> diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
>> index 7fb33f81788..b4736804eda 100644
>> --- a/gcc/fortran/expr.cc
>> +++ b/gcc/fortran/expr.cc
>> @@ -1539,6 +1539,7 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
>> mpz_init_set_ui (delta_mpz, one);
>> mpz_init_set_ui (nelts, one);
>> mpz_init (tmp_mpz);
>> +  mpz_init (ptr);
>> 
>> /* Do the initialization now, so that we can cleanup without
>>keeping track of where we were.  */
>> @@ -1682,7 +1683,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
>> mpz_mul (delta_mpz, delta_mpz, tmp_mpz);
>>   }
>> 
>> -  mpz_init (ptr);
>> cons = gfc_constructor_first (base);
>> 
>> /* Now clock through the array reference, calculating the index in
>> @@ -1735,7 +1735,8 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
>>   "at %L requires an increase of the allowed %d "
>>   "upper limit.  See %<-fmax-array-constructor%> "
>>   "option", >where, flag_max_array_constructor);
>> -  return false;
>> +  t = false;
>> +  goto cleanup;
>>  }
>> 
>> cons = gfc_constructor_lookup (base, limit);
>> @@ -1750,8 +1751,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
>> gfc_copy_expr (cons->expr), NULL);
>>   }
>> 
>> -  mpz_clear (ptr);
>> -
>>   cleanup:
>> 
>> mpz_clear (delta_mpz);
>> @@ -1765,6 +1764,7 @@ cleanup:
>> mpz_clear (ctr[d]);
>> mpz_clear (stride[d]);
>>   }
>> +  mpz_clear (ptr);
>> gfc_constructor_free (base);
>> return t;
>>   }
>> diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc
>> index ecf0e3558df..d1f06335e79 100644
>> --- a/gcc/fortran/simplify.cc
>> +++ b/gcc/fortran/simplify.cc
>> @@ -6866,6 +6866,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
>> *shape_exp,
>>gfc_error ("The SHAPE array for the RESHAPE intrinsic at %L has a "
>>   "negative value %d for dimension %d",
>>   _exp->where, shape[rank], rank+1);
>> +  mpz_clear (index);
>>return _bad_expr;
>>  }
>> 
>> @@ -6889,6 +6890,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
>> *shape_exp,
>>  {
>>gfc_error ("Shapes of ORDER at %L and SHAPE at %L are different",
>>   _exp->where, _exp->where);
>> +  mpz_clear (index);
>>return _bad_expr;
>>  }
>> 
>> @@ -6902,6 +6904,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
>> *shape_exp,
>>  {
>>gfc_error ("Sizes of ORDER at %L and SHAPE at %L are different",
>>

Re: [PATCH 2/2] Fortran: Fix memory leak in gfc_add_include_path [PR68800]

[Bernhard Reutner-Fischer via Fortran]

ping?

libcpp maintainers, is the helper in incpath.* ok?

fortraners,
Do you prefer a rogue, local forward declaration, or is the
introduction of that trivial wrapper ok? I don't think pulling in cpp.h
from f95-lang.cc is desirable, but i can do that if you all think
that's preferred.

cover-letter:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583522.html

gcc/incpath.* bits (i guess that'd be for the libcpp maintainers):
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583520.html

fortran bits:
https://gcc.gnu.org/pipermail/gcc-patches/2021-November/583521.html

thanks,

On Sun, 7 Nov 2021 02:38:21 +0100
Bernhard Reutner-Fischer  wrote:

> On Sat, 6 Nov 2021 20:22:53 +0100
> Harald Anlauf  wrote:
> 
> > Hi Bernhard,
> > 
> > I cannot comment on the gcc/ parts, but
> >   
> 
> > > diff --git a/gcc/fortran/cpp.c b/gcc/fortran/cpp.c
> > > index e86386c8b17..04fe8fe460b 100644
> > > --- a/gcc/fortran/cpp.c
> > > +++ b/gcc/fortran/cpp.c
> > > @@ -728,12 +728,20 @@ gfc_cpp_done (void)
> > > cpp_clear_file_cache (cpp_in);
> > >   }
> > 
> > why do you introduce a wrapper for something outside of fortran
> > that is used only once,
> >   
> > > -/* PATH must be malloc-ed and NULL-terminated.  */
> > > +/* Free all cpp include dirs.  */
> > > +void
> > > +gfc_cpp_free_cpp_dirs (void)
> > > +{
> > > +  free_cpp_dirs ();
> > > +}  
> 
> > > diff --git a/gcc/fortran/cpp.h b/gcc/fortran/cpp.h
> > > index 44644a2a333..963b9a9c89e 100644
> > > --- a/gcc/fortran/cpp.h
> > > +++ b/gcc/fortran/cpp.h
> > > @@ -46,6 +46,7 @@ void gfc_cpp_post_options (bool);
> > >   bool gfc_cpp_preprocess (const char *source_file);
> > >
> > >   void gfc_cpp_done (void);
> > > +void gfc_cpp_free_cpp_dirs (void);
> > >
> > >   void gfc_cpp_add_include_path (char *path, bool user_supplied);
> > >   void gfc_cpp_add_include_path_after (char *path, bool user_supplied);
> > > diff --git a/gcc/fortran/f95-lang.c b/gcc/fortran/f95-lang.c
> > > index 58dcaf01d75..ec4c2cf01d9 100644
> > > --- a/gcc/fortran/f95-lang.c
> > > +++ b/gcc/fortran/f95-lang.c
> > > @@ -275,7 +275,7 @@ gfc_finish (void)
> > > gfc_cpp_done ();
> > > gfc_done_1 ();
> > > gfc_release_include_path ();
> > > -  return;
> > 
> > namely here?
> >   
> > > +  gfc_cpp_free_cpp_dirs ();
> > >   }
> > 
> > Why not call free_cpp_dirs () here directly, omit all unnecessary
> > stuff, and maybe only add a brief comment here?  
> 
> cpp.c includes incpath.h, f95-lang.c does not and should not.
> So the cleanest thing is to keep the cpp handling in cpp.[ch] and have
> the language frontend call into it's cpp bits.
> 
> It would be rather rogue to
> extern void free_cpp_dirs (void);
> in f95-lang.c and directly call it in gfc_finish, i'd say?
> 
> thanks,

[PATCH 3/3] Fortran: Fix mpz and mpfr memory leaks

[Bernhard Reutner-Fischer via Fortran]

From: Bernhard Reutner-Fischer 

Cc: fortran@gcc.gnu.org

gcc/fortran/ChangeLog:

* array.cc (gfc_ref_dimen_size): Free mpz memory before ICEing.
* expr.cc (find_array_section): Fix mpz memory leak.
* simplify.cc (gfc_simplify_reshape): Fix mpz memory leaks in
error paths.
(gfc_simplify_set_exponent): Fix mpfr memory leak.
---
 gcc/fortran/array.cc| 3 +++
 gcc/fortran/expr.cc | 8 
 gcc/fortran/simplify.cc | 7 ++-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index be5eb8b6a0f..8b1e816a859 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -2541,6 +2541,9 @@ gfc_ref_dimen_size (gfc_array_ref *ar, int dimen, mpz_t 
*result, mpz_t *end)
   return t;
 
 default:
+  mpz_clear (lower);
+  mpz_clear (stride);
+  mpz_clear (upper);
   gfc_internal_error ("gfc_ref_dimen_size(): Bad dimen_type");
 }
 
diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc
index 7fb33f81788..b4736804eda 100644
--- a/gcc/fortran/expr.cc
+++ b/gcc/fortran/expr.cc
@@ -1539,6 +1539,7 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
   mpz_init_set_ui (delta_mpz, one);
   mpz_init_set_ui (nelts, one);
   mpz_init (tmp_mpz);
+  mpz_init (ptr);
 
   /* Do the initialization now, so that we can cleanup without
  keeping track of where we were.  */
@@ -1682,7 +1683,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
   mpz_mul (delta_mpz, delta_mpz, tmp_mpz);
 }
 
-  mpz_init (ptr);
   cons = gfc_constructor_first (base);
 
   /* Now clock through the array reference, calculating the index in
@@ -1735,7 +1735,8 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
 "at %L requires an increase of the allowed %d "
 "upper limit.  See %<-fmax-array-constructor%> "
 "option", >where, flag_max_array_constructor);
- return false;
+ t = false;
+ goto cleanup;
}
 
   cons = gfc_constructor_lookup (base, limit);
@@ -1750,8 +1751,6 @@ find_array_section (gfc_expr *expr, gfc_ref *ref)
   gfc_copy_expr (cons->expr), NULL);
 }
 
-  mpz_clear (ptr);
-
 cleanup:
 
   mpz_clear (delta_mpz);
@@ -1765,6 +1764,7 @@ cleanup:
   mpz_clear (ctr[d]);
   mpz_clear (stride[d]);
 }
+  mpz_clear (ptr);
   gfc_constructor_free (base);
   return t;
 }
diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc
index ecf0e3558df..d1f06335e79 100644
--- a/gcc/fortran/simplify.cc
+++ b/gcc/fortran/simplify.cc
@@ -6866,6 +6866,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
*shape_exp,
  gfc_error ("The SHAPE array for the RESHAPE intrinsic at %L has a "
 "negative value %d for dimension %d",
 _exp->where, shape[rank], rank+1);
+ mpz_clear (index);
  return _bad_expr;
}
 
@@ -6889,6 +6890,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
*shape_exp,
{
  gfc_error ("Shapes of ORDER at %L and SHAPE at %L are different",
 _exp->where, _exp->where);
+ mpz_clear (index);
  return _bad_expr;
}
 
@@ -6902,6 +6904,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
*shape_exp,
{
  gfc_error ("Sizes of ORDER at %L and SHAPE at %L are different",
 _exp->where, _exp->where);
+ mpz_clear (index);
  return _bad_expr;
}
 
@@ -6918,6 +6921,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
*shape_exp,
 "in the range [1, ..., %d] for the RESHAPE intrinsic "
 "near %L", order[i], _exp->where, rank,
 _exp->where);
+ mpz_clear (index);
  return _bad_expr;
}
 
@@ -6926,6 +6930,7 @@ gfc_simplify_reshape (gfc_expr *source, gfc_expr 
*shape_exp,
{
  gfc_error ("ORDER at %L is not a permutation of the size of "
 "SHAPE at %L", _exp->where, _exp->where);
+ mpz_clear (index);
  return _bad_expr;
}
  x[order[i]] = 1;
@@ -7408,7 +7413,7 @@ gfc_simplify_set_exponent (gfc_expr *x, gfc_expr *i)
   exp2 = (unsigned long) mpz_get_d (i->value.integer);
   mpfr_mul_2exp (result->value.real, frac, exp2, GFC_RND_MODE);
 
-  mpfr_clears (absv, log2, pow2, frac, NULL);
+  mpfr_clears (exp, absv, log2, pow2, frac, NULL);
 
   return range_check (result, "SET_EXPONENT");
 }
-- 
2.30.2

Re: F77 indexed file support

[Bernhard Reutner-Fischer via Fortran]

On 7 March 2023 23:18:58 CET, Roland Hughes via Fortran  
wrote:

[ snip namelist IO ]

>Btw, is there a "search" utility for the archives or do I have to pull down 
>all of the zip files, unzip into directory, and grep to look for stuff like 
>this? I'm guessing it has come up before.

Indeed we have
https://inbox.sourceware.org/fortran/

along the traditional pipermail ml interface.

thanks,

Re: [PATCH][stage1] Remove conditionals around free()

[Bernhard Reutner-Fischer via Fortran]

On 2 March 2023 02:23:10 CET, Jerry D  wrote:
>On 3/1/23 4:07 PM, Steve Kargl via Fortran wrote:
>> On Wed, Mar 01, 2023 at 10:28:56PM +0100, Bernhard Reutner-Fischer via 
>> Fortran wrote:
>>>   libgfortran/caf/single.c |6 ++
>>>   libgfortran/io/async.c   |6 ++
>>>   libgfortran/io/format.c  |3 +--
>>>   libgfortran/io/transfer.c|6 ++
>>>   libgfortran/io/unix.c|3 +--
>> 
>> The Fortran ones are OK.
>> 
>
>The only question I have: Is free posix compliant on all platforms?
>
>For example ming64 or mac?  It seems sometimes we run into things like this 
>once in a while.

I think we have the -liberty to cater even for non compliant systems either 
way, if you please excuse the pun. That's not an excuse on POSIX systems, imho.

>
>Otherwise I have no issue at all.  It is a lot cleaner.
>
>Jerry

Re: [PATCH, gfortran] Escalate failure when Hollerith constant to real conversion fails [PR103628]

[Bernhard Reutner-Fischer via Fortran]

On Wed, 1 Mar 2023 07:39:40 -0800
Steve Kargl via Gcc-patches  wrote:

> In fact, Hollerith should be hidden behind a -fallow-hollerith
> option and added to -std=legacy.

While i'd be all for that, in my mind this will block off literally all
consultants and quite some scientists unless we error out
with a specific hint to an option that re-enable this.

So yea, but please forgivingly for the decades to come?

HTH,

Re: [PATCH, gfortran] Escalate failure when Hollerith constant to real conversion fails [PR103628]

[Bernhard Reutner-Fischer via Fortran]

On Wed, 1 Mar 2023 10:40:15 +0100
Tobias Burnus  wrote:

> Hi,
> 
> Please CC fortran@gcc for Fortran patches.

> > --- a/gcc/fortran/target-memory.cc
> > +++ b/gcc/fortran/target-memory.cc
> > @@ -417,10 +417,13 @@ gfc_interpret_float (int kind, unsigned char *buffer, 
> > size_t buffer_size,
> >   {
> > gfc_set_model_kind (kind);
> > mpfr_init (real);
> > -  gfc_conv_tree_to_mpfr (real,
> > -  native_interpret_expr (gfc_get_real_type (kind),
> > - buffer, buffer_size));
> >
> > +  tree source  = native_interpret_expr (gfc_get_real_type (kind), buffer,

s/source  = native/source = native/

additionally to moving mpfr_init around and the other comments.
Please send an updated, regtested patch?
thanks && cheers,

Re: [PATCH][stage1] Remove conditionals around free()

[Bernhard Reutner-Fischer via Fortran]

On Wed, 1 Mar 2023 14:59:44 -0800
Andrew Pinski  wrote:

> On Wed, Mar 1, 2023 at 1:31 PM Bernhard Reutner-Fischer via Fortran
>  wrote:
> >
> > Hi!
> >
> > Mere cosmetics.
> >
> > - if (foo != NULL)
> > free (foo);
> >
> > With the caveat that coccinelle ruins replacement whitespace or i'm
> > uneducated enough to be unable to _not_ run the diff through
> >  sed -e 's/^+\([[:space:]]*\)free(/+\1free (/'
> > at least. If anybody knows how to improve replacement whitespace,
> > i'd be interrested but didn't look nor ask. ISTM that leading
> > whitespace is somewhat ruined, too, so beware (8 spaces versus tab as
> > far as i have spot-checked).
> >
> > Would touch
> >  gcc/ada/rtinit.c |3 +--  
> 
> 

It's funny how you apparently did not comment that hunk in the end ;)

> >  intl/bindtextdom.c   |3 +--
> >  intl/loadmsgcat.c|6 ++
> >  intl/localcharset.c  |3 +--  
> 
> intl is imported from glibc, though I don't know we have updated it in
> recent years from glibc.

i don't think we did, overdue, as we (probably) all know.
OTOH i'm thankful that we don't have submodules but a plain, manageable
repo. Of course that comes with a burden, which is nil if ignored
throughout. Doesn't always pay out too well longterm if nobody
(voluntarily) is in due charge.

> >  zlib/contrib/minizip/unzip.c |2 +-
> >  zlib/contrib/minizip/zip.c   |2 +-
> >  zlib/examples/enough.c   |6 ++
> >  zlib/examples/gun.c  |2 +-
> >  zlib/examples/gzjoin.c   |3 +--
> >  zlib/examples/gzlog.c|6 ++  
> 
> zlib is definitely imported from zlib upstream.
> So it might be good to check if we could import a new version and see
> if it still works instead.

From a meta POV, i wonder where the trailing space in the subject comes
from, looking at e.g.:
https://gcc.gnu.org/pipermail/gcc-patches/2023-March/date.html#613110
I think and hope that the newer(?) ones by
https://inbox.sourceware.org/gcc-patches/?t=xyz do not exhibit these
invented trailing blanks nobody ever wrote for real, does it.

Thanks for reminding me of intl and it's outdatedness, although i
certainly don't have ambition to do anything about it for sure.
I didn't care 15 or 20 years ago and nowadays i'd call that attitude a
tradition, at least ATM ;) TBH i initially had only considered gcc/ but
somehow found that unfair. Great idea that inclusion was.

thanks,

> > 4) i most likely will not remember to split it apart and send proper
> >patches, tested patches, in stage 1 to maintainers proper, so if
> >anyone feels like pursuing this, be my guest. I thought i'd just
> >mention it.
> >
> > cheers,  

Re: [PATCH][stage1] Remove conditionals around free()

[Bernhard Reutner-Fischer via Fortran]

On Wed, 1 Mar 2023 22:28:56 +0100
Bernhard Reutner-Fischer  wrote:

> Remarks:
> 1) We should do this in if-conversion (?) on our own.
>I suppose. Independently of -fdelete-null-pointer-checks

and iff we can prove that ptr was NULL when passed to free(ptr) then we
can elide the call, of course. Likewise for realloc(ptr, 0), obviously.
[or reallocarray -- yikes -- if nmemb == 0 || size == 0]

But that would probably be a ranger call in DCE, i guess. Didn't look.
thanks,

[PATCH][stage1] Remove conditionals around free()

[Bernhard Reutner-Fischer via Fortran]

Hi!

Mere cosmetics.

- if (foo != NULL)
free (foo);

With the caveat that coccinelle ruins replacement whitespace or i'm
uneducated enough to be unable to _not_ run the diff through
 sed -e 's/^+\([[:space:]]*\)free(/+\1free (/'
at least. If anybody knows how to improve replacement whitespace,
i'd be interrested but didn't look nor ask. ISTM that leading
whitespace is somewhat ruined, too, so beware (8 spaces versus tab as
far as i have spot-checked).

Would touch
 gcc/ada/rtinit.c |3 +--
 intl/bindtextdom.c   |3 +--
 intl/loadmsgcat.c|6 ++
 intl/localcharset.c  |3 +--
 libbacktrace/xztest.c|9 +++--
 libbacktrace/zstdtest.c  |9 +++--
 libbacktrace/ztest.c |9 +++--
 libgfortran/caf/single.c |6 ++
 libgfortran/io/async.c   |6 ++
 libgfortran/io/format.c  |3 +--
 libgfortran/io/transfer.c|6 ++
 libgfortran/io/unix.c|3 +--
 libgo/runtime/go-setenv.c|6 ++
 libgo/runtime/go-unsetenv.c  |3 +--
 libgomp/target.c |3 +--
 libiberty/concat.c   |3 +--
 zlib/contrib/minizip/unzip.c |2 +-
 zlib/contrib/minizip/zip.c   |2 +-
 zlib/examples/enough.c   |6 ++
 zlib/examples/gun.c  |2 +-
 zlib/examples/gzjoin.c   |3 +--
 zlib/examples/gzlog.c|6 ++

coccinelle script and invocation inline.
Would need to be split for the respective maintainers and run through
mklog with subject changelog and should of course be compiled and
tested before that.

Remarks:
1) We should do this in if-conversion (?) on our own.
   I suppose. Independently of -fdelete-null-pointer-checks
2) Maybe not silently, but raise language awareness nowadays.
   By now it's been a long time since this was first mandated.
3) fallout from looking at something completely different
4) i most likely will not remember to split it apart and send proper
   patches, tested patches, in stage 1 to maintainers proper, so if
   anyone feels like pursuing this, be my guest. I thought i'd just
   mention it.

cheers,
# cat ~/coccinelle/free-without-if-null.0.cocci ; echo EOF
// POSIX: free(NULL) is perfectly valid
// quote: If ptr is a null pointer, no action shall occur.
@ rule1 @
expression e;
@@

- if (e != NULL)
-  { free(e); }
+ free (e);

EOF
# find ./ \( -name "*.[ch]" -o -name "*.cpp" \) -a \( ! -path "./gcc/testsuite/*" -a ! -path "./gcc/contrib/*" \) -exec spatch --sp-file ~/coccinelle/free-without-if-null.0.cocci --in-place 
diff --git a/gcc/ada/rtinit.c b/gcc/ada/rtinit.c
index f1607b3c8b0..bbf02b1c2ae 100644
--- a/gcc/ada/rtinit.c
+++ b/gcc/ada/rtinit.c
@@ -481,8 +481,7 @@ __gnat_runtime_initialize (int install_handler)
 
 		 FindClose (hDir);
 
-		 if (dir != NULL)
-		   free (dir);
+		 free (dir);
 		   }
 	   }
 	 else
diff --git a/intl/bindtextdom.c b/intl/bindtextdom.c
index 6faac5756a3..c71c728401b 100644
--- a/intl/bindtextdom.c
+++ b/intl/bindtextdom.c
@@ -204,8 +204,7 @@ set_binding_values (domainname, dirnamep, codesetp)
 
 		  if (__builtin_expect (result != NULL, 1))
 		{
-		  if (binding->codeset != NULL)
-			free (binding->codeset);
+		  free (binding->codeset);
 
 		  binding->codeset = result;
 		  binding->codeset_cntr++;
diff --git a/intl/loadmsgcat.c b/intl/loadmsgcat.c
index 536ee12621d..e55d095ec56 100644
--- a/intl/loadmsgcat.c
+++ b/intl/loadmsgcat.c
@@ -1273,8 +1273,7 @@ _nl_load_domain (domain_file, domainbinding)
   /* This is an invalid revision.  */
 invalid:
   /* This is an invalid .mo file.  */
-  if (domain->malloced)
-	free (domain->malloced);
+  free (domain->malloced);
 #ifdef HAVE_MMAP
   if (use_mmap)
 	munmap ((caddr_t) data, size);
@@ -1307,8 +1306,7 @@ _nl_unload_domain (domain)
 
   _nl_free_domain_conv (domain);
 
-  if (domain->malloced)
-free (domain->malloced);
+  free (domain->malloced);
 
 # ifdef _POSIX_MAPPED_FILES
   if (domain->use_mmap)
diff --git a/intl/localcharset.c b/intl/localcharset.c
index 8ece6e39f69..8d34dcb0918 100644
--- a/intl/localcharset.c
+++ b/intl/localcharset.c
@@ -199,8 +199,7 @@ get_charset_aliases ()
 	}
 	}
 
-  if (file_name != NULL)
-	free (file_name);
+  free (file_name);
 
 #else
 
diff --git a/libbacktrace/xztest.c b/libbacktrace/xztest.c
index ed90066470a..6f4a5c73a00 100644
--- a/libbacktrace/xztest.c
+++ b/libbacktrace/xztest.c
@@ -479,12 +479,9 @@ test_large (struct backtrace_state *state ATTRIBUTE_UNUSED)
   printf ("FAIL: lzma large\n");
   ++failures;
 
-  if (orig_buf != NULL)
-free (orig_buf);
-  if (compressed_buf != NULL)
-free (compressed_buf);
-  if (uncompressed_buf != NULL)
-free (uncompressed_buf);
+  free (orig_buf);
+  free (compressed_buf);
+  free (uncompressed_buf);
 
 #else /* !HAVE_LIBLZMA */
 
diff --git a/libbacktrace/zstdtest.c b/libbacktrace/zstdtest.c
index 1b4158a50eb..8a0b27977b5 100644
--- 

drop -fdump-fortran-global ? [was: Re: [PATCH,FORTRAN] Fix memory leak of gsymbol]

[Bernhard Reutner-Fischer via Fortran]

On Sun, 31 Oct 2021 21:25:44 +0100
Bernhard Reutner-Fischer  wrote:

> On Sun, 31 Oct 2021 20:46:07 +0100
> Harald Anlauf  wrote:
> 
> > Am 30.10.21 um 23:51 schrieb Bernhard Reutner-Fischer via Fortran:  
> > >>> The only caller is translate_all_program_units.
> > >>> Since we free only module gsyms, even -fdump-fortran-global is
> > >>> unaffected by this, fwiw.
> > >
> > > AFAICS we do not have a test for -fdump-fortran-global
> > > Do we want to add one, would the attached be OK?
> > 
> > This doesn't seem to test anything new or changed, or a bug fixed.
> > I get the same result for all version from 9 to 12-mainline.
> > So as is it seems pointless.  
> 
> Yes indeed, it just adds coverage to that functionality which we did not
> exercise before.
> TBH i only found that option when looking around
> translate_all_program_units. I've never actually used that option
> myself and cannot imagine how it is useful at all :)
> 
> Dropped the testcase.
> Thanks for your comment!

The rest of 5c6aa9a8919cbf0dcf3c375f51012720bfb5f3a1 is fine, but
should we really keep the option, if we don't even test basics and if it
was more a specific debug dump, from the looks?

Thomas?

thanks,

Re: Support for NOINLINE attribute

[Bernhard Reutner-Fischer via Fortran]

Hi Rimvydas!

On Sat, 18 Feb 2023 21:35:47 +0100
Bernhard Reutner-Fischer  wrote:

> On Fri, 10 Feb 2023 07:42:47 +0200
> Rimvydas Jasinskas via Fortran  wrote:
> 
> > * decl.cc: Add EXT_ATTR_NOINLINE, EXT_ATTR_NORETURN, EXT_ATTR_WEAK.
> > * gfortran.h (ext_attr_id_t): Ditto.  
> 
> We had that discussion recently here..
> Which of these are required to be recorded to the module and why,
> exactly? Please elaborate.
> 
> thanks,

The aforementioned discussion was here:
https://gcc.gnu.org/pipermail/fortran/2022-November/058542.html

So, again, please elaborate why you need to store each of NOINLINE,
NORETURN, WEAK in the module?

thanks

Re: [PATCH 1/2] symtab: also change RTL decl name

[Bernhard Reutner-Fischer via Fortran]

On Sun, 19 Feb 2023 03:29:43 +0100
Bernhard Reutner-Fischer  wrote:

> But i've seen in C++ too that there are GC dangles like here.
s/like here//;# not true in this case. Those were GC marks for names
elsewhere.

To be fair, i'd have made lhd_overwrite_decl_assembler_name a
(where supported) weakref to lhd_type_promotes_to

Re: [PATCH 1/2] symtab: also change RTL decl name

[Bernhard Reutner-Fischer via Fortran]

On Tue, 22 Nov 2022 12:54:01 +0100
Jan Hubicka  wrote:

> > > I am not quite sure how safe this is.  We generally produce DECL_RTL
> > > when we produce assembly file.  So if DECL_RTL is set then we probably
> > > already output the original function name and it is too late to change
> > > it.  
> > 
> > AFAICS we make_decl_rtl in the fortran FE in trans_function_start.  
> 
> I see, it may be a relic of something that is no longer necessary.  Can
> you see why one needs DECL_RTL so early?

No.

I think this is a ward, isn't it.

diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
index ff64588b9a8..a801e66fb11 100644
--- a/gcc/fortran/trans-decl.cc
+++ b/gcc/fortran/trans-decl.cc
@@ -2922,7 +2922,7 @@ trans_function_start (gfc_symbol * sym)
 }
 
   /* Create RTL for function definition.  */
-  make_decl_rtl (fndecl);
+  //make_decl_rtl (fndecl);
 
   allocate_struct_function (fndecl, false);

But we have that alot, with varous workarounds near
lhd_set_decl_assembler_name.
gcc.gnu.org/pr94223 comes to mind, but that was counters.
But i've seen in C++ too that there are GC dangles like here.

5f3682ffcef162363b783eb9ee702debff489fa8 a.k.a
https://gcc.gnu.org/legacy-ml/gcc-patches/2017-11/msg01340.html

ah lhd_overwrite_decl_assembler_name.

So.. why do we have that, again?
Per default it doesn't look much if there are clones or an ifunc
dispatcher, does it.

Re: Team Collaboration Considerations

[Bernhard Reutner-Fischer via Fortran]

On 19 January 2023 20:03:38 CET, NightStrike  wrote:

>The problem is that patch tracking is unsustainable. You could go the other
>way and have a patch tracker automatically echo messages to the mailing
>list.

Currently it's the other way round. Patchwork collects the patches sent to the 
list. Everybody is free to administrate her patches in Patchwork iff that is 
desired.

Or do the same but through bugzilla.

I am aware that all this might not be the way you envision things to work, of 
course.

Re: git out-of-order commit (was Re: [PATCH] Fortran: Remove unused declaration)

[Bernhard Reutner-Fischer via Fortran]

On 19 January 2023 20:39:08 CET, Jason Merrill  wrote:
>On Sat, Nov 12, 2022 at 4:24 PM Harald Anlauf via Gcc-patches
> wrote:
>>
>> Am 12.11.22 um 22:05 schrieb Bernhard Reutner-Fischer via Gcc-patches:
>> > This function definition was removed years ago, remove it's prototype.
>> >
>> > gcc/fortran/ChangeLog:
>> >
>> >   * gfortran.h (gfc_check_include): Remove declaration.
>> > ---
>> >   gcc/fortran/gfortran.h | 1 -
>> >   1 file changed, 1 deletion(-)
>> > ---
>> > Regtests cleanly, ok for trunk?
>> >
>> > diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
>> > index c4deec0d5b8..ce3ad61bb52 100644
>> > --- a/gcc/fortran/gfortran.h
>> > +++ b/gcc/fortran/gfortran.h
>> > @@ -3208,7 +3208,6 @@ int gfc_at_eof (void);
>> >   int gfc_at_bol (void);
>> >   int gfc_at_eol (void);
>> >   void gfc_advance_line (void);
>> > -int gfc_check_include (void);
>> >   int gfc_define_undef_line (void);
>> >
>> >   int gfc_wide_is_printable (gfc_char_t);
>>
>> OK, thanks.
>
>Somehow this was applied with a CommitDate in 2021, breaking scripts
>that assume monotonically increasing CommitDate.  Anyone know how that
>could have happened?

Sorry for that.
I think i cherry-picked this commit to master before pushing it, not 100% sure 
though.
What shall we do now?

Re: Team Collaboration Considerations

[Bernhard Reutner-Fischer via Fortran]

On 19 January 2023 13:52:55 CET, NightStrike via Fortran  
wrote:

>You can, and people naturally do this, and I think it's great, but
>there's usually a response from someone saying "post that to the
>mailing list instead".

The mailing list has a 20-30 year history with reasoning about what currently 
is in the tree. I do think it is valuable to reason about patches publically 
for others to see. And I'm aware that this might not be regarded fancy nowadays 
by everyone.

But that does not mean that using other means to collaborate should not be used 
by some. Be it comp.lang.fortran, a webchat.oftc, or other means, that's all 
fine of course.

patches currently are handled differently, but I don't think that is a problem 
isn't it.
Just post final patches to the list as long as that is regarded the way to do 
final review and document approval.

cheers,

Re: [PATCH 2/2] Fortran: add attribute target_clones

[Bernhard Reutner-Fischer via Fortran]

On Mon, 21 Nov 2022 20:13:40 +0100
Mikael Morin  wrote:

> Hello,
> 
> Le 09/11/2022 à 20:02, Bernhard Reutner-Fischer via Fortran a écrit :
> > Hi!
> > 
> > Add support for attribute target_clones:
> > !GCC$ ATTRIBUTES target_clones("arch1", "arch3","default") :: mysubroutine

> > +/* Internal helper to parse attribute argument list.
> > +   If REQUIRE_STRING is true, then require a string.
> > +   If ALLOW_MULTIPLE is true, allow more than one arg.
> > +   If multiple arguments are passed, require braces around them.
> > +   Returns a tree_list of arguments or NULL_TREE.  */
> > +static tree
> > +gfc_match_gcc_attribute_args (bool require_string, bool allow_multiple)

> > +   do {

> > +   } while (num_quotes % 2 && gfc_match_eos () != MATCH_YES);  
> The do-while loops are wrongly indented.
> It should be:
>do
>  {
>...
>  }
>while (...)

oops, right.

> > +   tree str = build_string (pos, name);
> > +   /* Compare with c-family/c-common.cc: fix_string_type.  */
> > +   tree i_type = build_index_type (size_int (pos));
> > +   tree a_type = build_array_type (char_type_node, i_type);
> > +   TREE_TYPE (str) = a_type;
> > +   TREE_READONLY (str) = 1;
> > +   TREE_STATIC (str) = 1;
> > +   attr_arg = build_tree_list (NULL_TREE, str);
> > +   attr_args = chainon (attr_args, attr_arg);  
> Same comment as for the flatten attribute:
> please no tree stuff out of the trans-*.cc files.

yes ok, noted. It's a pity in this context, where we purely pass a blob
on to the ME but ok.

> This includes gfortran.h, so the attribute arguments need to be carried 
> around using the front-end structures (gfc_actual_arglist for example).

That's a sane rule of thumb, yes.
Usually, the parser deals with language grammar and not with pure
passthrough remarks, so that's fair. Not so much in the case of such
attribs but i see your point :)
 
> > +  if (allow_multiple && gfc_match_char (')') != MATCH_YES)
> > +{
> > +  gfc_error ("expected ')' at %C");
> > +  return NULL_TREE;
> > +}
> > +
> > +  return attr_args;
> > +}  
> I'm not sure this function need to do all the parsing manually.
> I would rather use gfc_match_actual_arglist, or maybe implement the 
> function as a wrapper around it.
> What is allowed here?  Are non-literal constants allowed, for example 
> parameter variables?  Is line continuation supported ?

Line continuation is supported i think.
Parameter variables supposedly are or should not be supported. Why would
you do that in the context of an attribute target decl?
Either way, if the ME does not find such an fndecl, it will complain
and ignore the attribute.
I don't understand non-literal constants in this context.
This very attribute applies to decls, so the existing code supposedly
matches a comma separated list of identifiers. The usual dollar-ok
caveats apply.

As to gfc_match_actual_arglist, probably.
target_clones has
+  { "target_clones",  1, -1, true, false, false, false,
+ dummy, NULL },
with tree-core.h struct attribute_spec, so
name, min=1, max=unbounded, decl_required=yes, ...ignore...

hence applies to functions and subroutines and the like. It does take an
unbounded list of strings, isa1, isa2, isa4, default. We could add
"default" unless seen, but i'd rather want it spelled out by the user
for the user is supposed to know what she's doing, as in c or c++.
The ME has code to sanity-check the attributes, including conflicting
(ME) attributes.

The reason why i contemplated with a separate parser was that for stuff
like regparm or sseregparm, you would want to require a single number
for the equivalent of

__attribute__((regparm(3),stdcall)

which you would provide in 2 separate !GCC$ attributes i assume.

> 
> Nothing (bad) to say about the rest, but there is enough to change with 
> the above comments.

Yes, many thanks for your comments.
I think there is no other non-intrusive way to pass the data through the
frontend. So for an acceptable way this means touching quite some spots
for every single ME attribute anybody would like to add in the future.
But that's how it is.

Re: [PATCH 1/2] Fortran: Cleanup struct ext_attr_t

[Bernhard Reutner-Fischer via Fortran]

On Mon, 21 Nov 2022 12:08:20 +0100
Mikael Morin  wrote:

> > * gfortran.h (struct ext_attr_t): Remove middle_end_name.
> > * trans-decl.cc (add_attributes_to_decl): Move building
> > tree_list to ...
> > * decl.cc (gfc_match_gcc_attributes): ... here. Add the attribute to
> > the tree_list for the middle end.
> >   
> I prefer to not do any middle-end stuff at parsing time, so I would 
> rather not do this change.
> Not OK.

Ok, that means we should filter-out those bits that we don't want to
write to the module (right?). We've plenty of bits left, more than Dave
Love would want to have added, i hope, so that should not be much of a
concern.

What that table really wants to say is whether or not this attribute
should be passed to the ME. Would it be acceptable to remove these
duplicate strings and just have a bool/char/int that is true if it
should be lowered (in trans-decl, as before)? But now i admit it's just
bikeshedding and we can as well leave it alone for now.. It was just a
though.

thanks,

Re: [PATCH 2/2] Fortran: Add attribute flatten

[Bernhard Reutner-Fischer via Fortran]

On Mon, 21 Nov 2022 12:24:11 +0100
Mikael Morin  wrote:

> > --- a/gcc/fortran/decl.cc
> > +++ b/gcc/fortran/decl.cc  
> (...)
> > @@ -11849,7 +11850,9 @@ gfc_match_gcc_attributes (void)
> > if (strcmp (name, ext_attr_list[id].name) == 0)
> >   break;
> >   
> > -  if (id == EXT_ATTR_LAST)
> > +  if (strcmp (name, "flatten") == 0)
> > +   known_attr0args = true; /* Handled below.  We do not need a bit.  */  
> 
> I don't see the point to have all the attributes needing a bit except 
> one that doesn't but needs a specific handling.
> What does it look like without the 1/2 patch and if one bit is also used 
> for flatten, like the other attributes?

I've changed target_clones not to use a bit locally because it's not
needed. From my understanding, we only need the bits for attributes
that change the calling convention or the caller(at least so far, but
that does make sense to me). Remember that we store these bits in the
module. Presumably because we have to make sure that a program/module
uses the correct calling convention for a module function annotated
with such an attribute (think cdecl, stdcall, fastcall, dllimport,
dllexport, or the non-implemented regparm, sseregparm) or for
attributes that otherwise influence the callers or callees (like
deprecated or no_arg_check).

Attributes like target_clones or flatten or (probably) optimize etc, do
not influence the callees, so we really do not need to store them in
the module.

Can you think of a reason to store them nevertheless?

> > +  else if (id == EXT_ATTR_LAST)
> > {
> >   gfc_error ("Unknown attribute in !GCC$ ATTRIBUTES statement at %C");
> >   return MATCH_ERROR;  
> 
> > diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi
> > index 06e4c8c00a1..be650f28b62 100644
> > --- a/gcc/fortran/gfortran.texi
> > +++ b/gcc/fortran/gfortran.texi
> > @@ -3280,6 +3280,14 @@ contains
> >   end module mymod
> >   @end smallexample
> >   
> > +@node flatten
> > +
> > +Procedures annotated with the @code{flatten} attribute have their
> > +callees inlined, if possible.  
> I'm not a native speaker, but I find this sentence confusing.
> The words of the gcc manual you are refering to seem more clear: "every 
> call inside the function is inlined, if possible".

Me neither and it was a bit too brief. I've changed this to:
Every call inside a procedure annotated with the @code{flatten} attribute
is inlined, if possible.  Please refer to
@ref{Top,,Common Function Attributes,gcc,Using the GNU Compiler Collection (GCC>
for details about the respective attribute.

Is that better?

That said, i think this whole attribute section in the manual is not
structured too well. I'd prefer to have a list of attributes like in the
"Common Function Attributes" section in the extend.texi.
Maybe it would be better to just start a new list of attributes at the
end of the current @subsection ATTRIBUTES directive, a subsubsection
with "Other attributes" and just itemize the new ones? We'd point
people to the Top docs once for further details and then just briefly
list the attributes we support. Would that be acceptable?

Many thanks for your comments!

> 
> > +Please refer to
> > +@ref{Top,,Common Function Attributes,gcc,Using the GNU Compiler Collection 
> > (GCC)}
> > +for details about the respective attribute.
> > +
> >   The attributes are specified using the syntax
> >   
> >   @code{!GCC$ ATTRIBUTES} @var{attribute-list} @code{::} 
> > @var{variable-list}  
> 

Re: [PATCH 1/2] symtab: also change RTL decl name

[Bernhard Reutner-Fischer via Fortran]

On Mon, 21 Nov 2022 20:02:49 +0100
Jan Hubicka  wrote:

> > Hi Honza, Ping.
> > Regtests cleanly for c,fortran,c++,ada,d,go,lto,objc,obj-c++
> > Ok?
> > I'd need this for attribute target_clones for the Fortran FE.  
> Sorry for delay here.
> > >  void
> > > @@ -303,6 +301,10 @@ symbol_table::change_decl_assembler_name (tree decl, 
> > > tree name)
> > >   warning (0, "%qD renamed after being referenced in assembly", decl);
> > >  
> > >SET_DECL_ASSEMBLER_NAME (decl, name);
> > > +  /* Set the new name in rtl.  */
> > > +  if (DECL_RTL_SET_P (decl))
> > > + XSTR (XEXP (DECL_RTL (decl), 0), 0) = IDENTIFIER_POINTER (name);  
> 
> I am not quite sure how safe this is.  We generally produce DECL_RTL
> when we produce assembly file.  So if DECL_RTL is set then we probably
> already output the original function name and it is too late to change
> it.

AFAICS we make_decl_rtl in the fortran FE in trans_function_start.

> 
> Also RTL is shared so changing it in-place is going to rewrite all the
> existing RTL expressions using it.
> 
> Why the DECL_RTL is produced for function you want to rename?

I think the fortran FE sets it quite early when lowering a function.
Later, when the ME creates the target_clones, it wants to rename the
initial function to initial_fun.default for the default target.
That's where the change_decl_assembler_name is called (only on the
decl).
But nobody changes the RTL name, so the ifunc (which should be the
initial, unchanged name) is properly emitted but
assemble_start_function uses the same, unchanged, initial fnname it
later obtains by get_fnname_from_decl which fetches the (wrong) initial
name where it should use the .default target name.
See
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/605081.html

I'm open to other suggestions to make this work in a different way, of
course. Maybe we're missing some magic somewhere that might share the
name between the fndecl and the RTL XSTR so the RTL is magically
updated by that single SET_ECL_ASSEMBLER_NAME in
change_decl_assembler_name? But i didn't quite see where that'd be?

thanks,

> Honza
> > > +
> > >if (alias)
> > >   {
> > > IDENTIFIER_TRANSPARENT_ALIAS (name) = 1;  
> >   

Re: [PATCH 1/2] symtab: also change RTL decl name

[Bernhard Reutner-Fischer via Fortran]

Hi Honza, Ping.
Regtests cleanly for c,fortran,c++,ada,d,go,lto,objc,obj-c++
Ok?
I'd need this for attribute target_clones for the Fortran FE.
thanks,

On Wed,  9 Nov 2022 20:02:24 +0100
Bernhard Reutner-Fischer  wrote:

> We were changing the ASSEMBLER_NAME of the function decl
> but not the name in DECL_RTL which is used as the function name
> fnname in rest_of_handle_final(). This led to using the old, wrong name
> for the attribute target default function when using target_clones.
> 
> Bootstrapped and regtested cleanly on x86_64-unknown-linux
> for c,c++,fortran,lto.
> Ok for trunk?
> 
> gcc/ChangeLog:
> 
>   * symtab.cc: Remove stray comment.
>   (symbol_table::change_decl_assembler_name): Also update the
>   name in DECL_RTL.
> 
> Cc: Jan Hubicka 
> ---
>  gcc/symtab.cc | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/gcc/symtab.cc b/gcc/symtab.cc
> index f2d96c0268b..2e20bf5fefc 100644
> --- a/gcc/symtab.cc
> +++ b/gcc/symtab.cc
> @@ -154,8 +154,6 @@ symbol_table::decl_assembler_name_equal (tree decl, 
> const_tree asmname)
>  }
>  
>  
> -/* Returns nonzero if P1 and P2 are equal.  */
> -
>  /* Insert NODE to assembler name hash.  */
>  
>  void
> @@ -303,6 +301,10 @@ symbol_table::change_decl_assembler_name (tree decl, 
> tree name)
>   warning (0, "%qD renamed after being referenced in assembly", decl);
>  
>SET_DECL_ASSEMBLER_NAME (decl, name);
> +  /* Set the new name in rtl.  */
> +  if (DECL_RTL_SET_P (decl))
> + XSTR (XEXP (DECL_RTL (decl), 0), 0) = IDENTIFIER_POINTER (name);
> +
>if (alias)
>   {
> IDENTIFIER_TRANSPARENT_ALIAS (name) = 1;

PING Re: [PATCH] Fortran: Remove double spaces in open() warning [PR99884]

[Bernhard Reutner-Fischer via Fortran]

yearly ping. Ok for trunk after re-regtesting?

thanks,

On Sun, 31 Oct 2021 13:57:46 +0100
Bernhard Reutner-Fischer  wrote:

> From: Bernhard Reutner-Fischer 
> 
> gcc/fortran/ChangeLog:
> 
>   PR fortran/99884
>   * io.c (check_open_constraints): Remove double spaces.
> ---
>  gcc/fortran/io.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/fortran/io.c b/gcc/fortran/io.c
> index fc97df79eca..9506f35008e 100644
> --- a/gcc/fortran/io.c
> +++ b/gcc/fortran/io.c
> @@ -2513,7 +2513,7 @@ check_open_constraints (gfc_open *open, locus *where)
> spec = "";
>   }
>  
> -  warn_or_error (G_("%s specifier at %L not allowed in OPEN statement 
> for "
> +  warn_or_error (G_("%sspecifier at %L not allowed in OPEN statement for 
> "
>"unformatted I/O"), spec, loc);
>  }
>  

Re: [PATCH 3/5] Fortran: Narrow return types [PR78798]

[Bernhard Reutner-Fischer via Fortran]

On 13 November 2022 21:29:50 CET, Harald Anlauf  wrote:

>Replacing "int" by "signed char" adds confusion and makes code
>less understandable, so I would oppose it, as we don't solve a
>real problem and rather add confusion.

Ok so consider the non-bool hunks dropped, they just fell out of my helper and 
I thought I'd ask.

I can send an updated patch during the weekend.

thanks,

Re: [PATCH 3/5] Fortran: Narrow return types [PR78798]

[Bernhard Reutner-Fischer via Fortran]

On Sun, 13 Nov 2022 12:13:26 +0200
Janne Blomqvist  wrote:

> On Sun, Nov 13, 2022 at 1:47 AM Bernhard Reutner-Fischer via Fortran
>  wrote:
> > --- a/gcc/fortran/arith.cc
> > +++ b/gcc/fortran/arith.cc
> > @@ -1135,7 +1135,7 @@ compare_complex (gfc_expr *op1, gfc_expr *op2)
> > strings.  We return -1 for a < b, 0 for a == b and 1 for a > b.
> > We use the processor's default collating sequence.  */
> >
> > -int
> > +signed char
> >  gfc_compare_string (gfc_expr *a, gfc_expr *b)
> >  {
> >size_t len, alen, blen, i;
> > @@ -1162,7 +1162,7 @@ gfc_compare_string (gfc_expr *a, gfc_expr *b)
> >  }  
> 
> Hmm, really? PR 78798 mentions changing int to bool, where
> appropriate, which I think is uncontroversial, but this?

Well we could leave this or all spots alone where a bool is
insufficient, if you prefer.

In the case of gfc_compare_string, the only user is simplify which only
looks at ge/gt/le/lt 0

[PATCH 4/5] value-range: Add as_string diagnostics helper

[Bernhard Reutner-Fischer via Fortran]

gcc/ChangeLog:

* value-range.cc (get_bound_with_infinite_markers): New static helper.
(irange::as_string): New definition.
* value-range.h: New declaration.

---
Provide means to print a value range to a newly allocated buffer.
The caller is responsible to free() the allocated memory.

Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
Ok for trunk?

Cc: Andrew MacLeod 
Cc: Aldy Hernandez 
---
 gcc/value-range.cc | 56 ++
 gcc/value-range.h  |  3 +++
 2 files changed, 59 insertions(+)

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index a855aaf626c..51cd9a38d90 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -3099,6 +3099,62 @@ debug (const value_range )
   fprintf (stderr, "\n");
 }
 
+/* Helper for irange::as_string().  Print a bound to an allocated buffer.  */
+static char *
+get_bound_with_infinite_markers (tree bound)
+{
+  tree type = TREE_TYPE (bound);
+  wide_int type_min = wi::min_value (TYPE_PRECISION (type), TYPE_SIGN (type));
+  wide_int type_max = wi::max_value (TYPE_PRECISION (type), TYPE_SIGN (type));
+
+  if (INTEGRAL_TYPE_P (type)
+  && !TYPE_UNSIGNED (type)
+  && TREE_CODE (bound) == INTEGER_CST
+  && wi::to_wide (bound) == type_min
+  && TYPE_PRECISION (type) != 1)
+return xstrdup ("-INF");
+  else if (TREE_CODE (bound) == INTEGER_CST
+  && wi::to_wide (bound) == type_max
+  && TYPE_PRECISION (type) != 1)
+return xstrdup ("+INF");
+  else
+return print_generic_expr_to_str (bound);
+}
+
+
+/* Return an irange as string. Return NULL on failure, an allocated
+   string on success.  */
+char *
+irange::as_string ()
+{
+  char *ret = NULL;
+  if (undefined_p() || varying_p () || m_num_ranges == 0)
+return ret;
+
+  for (unsigned i = 0; i < m_num_ranges; ++i)
+{
+  tree lb = m_base[i * 2];
+  tree ub = m_base[i * 2 + 1];
+  /* Construct [lower_bound,upper_bound].  */
+  char *lbs = get_bound_with_infinite_markers (lb);
+  char *ubs = get_bound_with_infinite_markers (ub);
+  /* Paranoia mode */
+  if (!lbs)
+   lbs = xstrdup ("");
+  if (!ubs)
+   ubs = xstrdup ("");
+
+  if (ret)
+   ret = reconcat (ret, ret, "[", lbs, ",", ubs, "]", NULL);
+  else
+   ret = concat ("[", lbs, ",", ubs, "]", NULL);
+
+  free (lbs);
+  free (ubs);
+}
+  return ret;
+}
+
 /* Create two value-ranges in *VR0 and *VR1 from the anti-range *AR
so that *VR0 U *VR1 == *AR.  Returns true if that is possible,
false otherwise.  If *AR can be represented with a single range
diff --git a/gcc/value-range.h b/gcc/value-range.h
index c87734dd8cd..76242e4bf45 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -160,6 +160,9 @@ public:
   wide_int get_nonzero_bits () const;
   void set_nonzero_bits (const wide_int_ref );
 
+  // For diagnostics.
+  char *as_string ();
+
   // Deprecated legacy public methods.
   tree min () const;   // DEPRECATED
   tree max () const;   // DEPRECATED
-- 
2.38.1

[PATCH 3/5] Fortran: Narrow return types [PR78798]

[Bernhard Reutner-Fischer via Fortran]

gcc/fortran/ChangeLog:

* arith.cc (compare_complex): Use narrower return type.
(gfc_compare_string): Likewise.
* arith.h (gfc_compare_string): Same.
(gfc_compare_with_Cstring): Ditto.
* array.cc (compare_bounds): Ditto.
(gfc_compare_array_spec): Likewise.
(is_constant_element): Likewise.
(gfc_constant_ac): Likewise.
* check.cc (dim_rank_check): Likewise.
* cpp.cc (gfc_cpp_init_options): Likewise.
(dump_macro): Likewise.
* cpp.h (gfc_cpp_handle_option): Likewise.
* dependency.cc (gfc_ref_needs_temporary_p): Likewise.
(gfc_check_argument_dependency): Likewise.
(gfc_check_fncall_dependency): Likewise.
(ref_same_as_full_array): Likewise.
* dependency.h (gfc_check_fncall_dependency): Likewise.
(gfc_dep_resolver): Likewise.
(gfc_are_equivalenced_arrays): Likewise.
* expr.cc (gfc_copy_ref): Likewise.
(gfc_kind_max): Likewise.
(numeric_type): Likewise.
* gfortran.h (gfc_at_end): Likewise.
(gfc_at_eof): Likewise.
(gfc_at_bol): Likewise.
(gfc_at_eol): Likewise.
(gfc_check_include): Likewise.
(gfc_define_undef_line): Likewise.
(gfc_wide_is_printable): Likewise.
(gfc_wide_is_digit): Likewise.
(gfc_wide_fits_in_byte): Likewise.
(get_c_kind): Likewise.
(gfc_find_sym_tree): Likewise.
(gfc_generic_intrinsic): Likewise.
(gfc_specific_intrinsic): Likewise.
(gfc_intrinsic_actual_ok): Likewise.
(gfc_has_vector_index): Likewise.
(gfc_numeric_ts): Likewise.
(gfc_impure_variable): Likewise.
(gfc_pure): Likewise.
(gfc_implicit_pure): Likewise.
(gfc_elemental): Likewise.
(gfc_pure_function): Likewise.
(gfc_implicit_pure_function): Likewise.
(gfc_compare_array_spec): Likewise.
(gfc_constant_ac): Likewise.
(gfc_expanded_ac): Likewise.
(gfc_check_digit): Likewise.
* intrinsic.cc (gfc_find_subroutine): Likewise.
(gfc_generic_intrinsic): Likewise.
(gfc_specific_intrinsic): Likewise.
* io.cc (compare_to_allowed_values): Likewise.
* misc.cc (gfc_done_2): Likewise.
* parse.cc: Likewise.
* parse.h (gfc_check_do_variable): Likewise.
* primary.cc (gfc_check_digit): Likewise.
* resolve.cc (resolve_structure_cons): Likewise.
(pure_stmt_function): Likewise.
(gfc_pure_function): Likewise.
(impure_stmt_fcn): Likewise.
(resolve_forall_iterators): Likewise.
(resolve_data): Likewise.
(gfc_impure_variable): Likewise.
(gfc_pure): Likewise.
(gfc_unset_implicit_pure): Likewise.
* scanner.cc (wide_is_ascii): Likewise.
(gfc_wide_toupper): Likewise.
(gfc_open_included_file): Likewise.
(gfc_at_end): Likewise.
(gfc_at_eof): Likewise.
(gfc_at_bol): Likewise.
(skip_comment_line): Likewise.
(gfc_gobble_whitespace): Likewise.
* symbol.cc (gfc_find_symtree_in_proc): Likewise.
* target-memory.cc (size_integer): Likewise.
(size_complex): Likewise.
* trans-array.cc: Likewise.
* trans-decl.cc (gfc_set_decl_assembler_name): Likewise.
* trans-types.cc (gfc_get_element_type): Likewise.
(gfc_add_field_to_struct): Likewise.
* trans-types.h (gfc_copy_dt_decls_ifequal): Likewise.
(gfc_return_by_reference): Likewise.
(gfc_is_nodesc_array): Likewise.
* trans.h (gfc_can_put_var_on_stack): Likewise.
---
Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
Ok for trunk?

Cc: fortran@gcc.gnu.org
---
 gcc/fortran/arith.cc |  4 +--
 gcc/fortran/arith.h  |  4 +--
 gcc/fortran/array.cc |  8 +++---
 gcc/fortran/check.cc |  2 +-
 gcc/fortran/cpp.cc   |  3 +--
 gcc/fortran/cpp.h|  2 +-
 gcc/fortran/dependency.cc|  8 +++---
 gcc/fortran/dependency.h |  6 ++---
 gcc/fortran/expr.cc  |  6 ++---
 gcc/fortran/gfortran.h   | 51 ++--
 gcc/fortran/intrinsic.cc |  6 ++---
 gcc/fortran/io.cc| 13 ++---
 gcc/fortran/misc.cc  |  2 +-
 gcc/fortran/parse.cc |  2 +-
 gcc/fortran/parse.h  |  2 +-
 gcc/fortran/primary.cc   |  4 +--
 gcc/fortran/resolve.cc   | 22 
 gcc/fortran/scanner.cc   | 20 +++---
 gcc/fortran/symbol.cc|  2 +-
 gcc/fortran/target-memory.cc |  6 ++---
 gcc/fortran/trans-array.cc   |  2 +-
 gcc/fortran/trans-decl.cc|  2 +-
 gcc/fortran/trans-types.cc   |  6 ++---
 gcc/fortran/trans-types.h|  6 ++---
 gcc/fortran/trans.h  |  2 +-
 25 files changed, 90 insertions(+), 101 deletions(-)

diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc
index fc9224ebc5c..55f35ea66be 100644
--- a/gcc/fortran/arith.cc

[PATCH 5/5] gimple: Add pass to note possible type demotions; IPA pro/demotion; DO NOT MERGE

[Bernhard Reutner-Fischer via Fortran]

gcc/ChangeLog:

* Makefile.in (OBJS): Add gimple-warn-types.o.
* passes.def: Add pass_warn_type_demotion.
* tree-pass.h (make_pass_warn_type_demotion): New declaration.
* gimple-warn-types.cc: New file.

gcc/c-family/ChangeLog:

* c.opt (Wtype-demotion): New.

---
DO NOT MERGE.
This is the script^Wpass to emit a warning if a function's return type
could potentially be narrowed.
What would probably be useful is to equip an IPA pass with that
knowledge and demote return types of functions that do not contribute to
an external interface to the smallest possible type. The idea is that if
a target determines late via targetm.calls.promote_prototypes to promote
return values, the target will ultimately have the final say about
return types while for non-exported functions we can narrow types for
size- or speed reasons as we see fit.

This hunk does not implement an IPA pass that would do anything really
useful but merely queries the ranger to see if there are possibilities
to demote a return type, any (!) return type.
For the IPA real thing, we'd want to notice that if a function returns a
singleton, the caller would just use that singleton and demote the
callee to void. And in the caller, we'd appropriately shift the callee's
return value to the required range/value.
The usual trouble makers come to mind: qsort helpers that insist on int
return codes (that we could extend to int).

As said, that's just for your amusement and is not meant to be merged.
---
 gcc/Makefile.in  |   1 +
 gcc/c-family/c.opt   |   6 +
 gcc/gimple-warn-types.cc | 441 +++
 gcc/passes.def   |   1 +
 gcc/tree-pass.h  |   1 +
 5 files changed, 450 insertions(+)
 create mode 100644 gcc/gimple-warn-types.cc

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index f672e6ea549..c6901ececd4 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1433,6 +1433,7 @@ OBJS = \
gimple-streamer-out.o \
gimple-walk.o \
gimple-warn-recursion.o \
+   gimple-warn-types.o \
gimplify.o \
gimplify-me.o \
godump.o \
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 63a300ecd7c..0b46669e2b7 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1005,6 +1005,12 @@ Wtemplates
 C++ ObjC++ Var(warn_templates) Warning
 Warn on primary template declaration.
 
+Wtype-demotion
+C ObjC C++ ObjC++ Var(warn_type_demotion) Warning LangEnabledBy(C ObjC C++ 
ObjC++, Wall)
+Warn if function return type could be narrowed or demoted.
+; function return type, parameter type, variable type.
+; if values used for a type indicate that the type could use a narrower mode.
+
 Wmissing-attributes
 C ObjC C++ ObjC++ Var(warn_missing_attributes) Warning LangEnabledBy(C ObjC 
C++ ObjC++,Wall)
 Warn about declarations of entities that may be missing attributes
diff --git a/gcc/gimple-warn-types.cc b/gcc/gimple-warn-types.cc
new file mode 100644
index 000..e0b7212a1bb
--- /dev/null
+++ b/gcc/gimple-warn-types.cc
@@ -0,0 +1,441 @@
+/* Pass to detect and issue warnings about possibly using narrower types.
+
+   Copyright (C) 2021-2022 Free Software Foundation, Inc.
+   Contributed by Bernhard Reutner-Fischer 
+
+   This file is part of GCC.
+
+   GCC is free software; you can redistribute it and/or modify it under
+   the terms of the GNU General Public License as published by the Free
+   Software Foundation; either version 3, or (at your option) any later
+   version.
+
+   GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or
+   FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+   for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with GCC; see the file COPYING3.  If not see
+   <http://www.gnu.org/licenses/>.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "tree.h"
+#include "gimple.h"
+#include "tree-pass.h"
+#include "pointer-query.h"
+#include "ssa.h"
+#include "gimple-pretty-print.h"
+#include "diagnostic-core.h"
+#include "fold-const.h"
+#include "gimple-iterator.h"
+#include "tree-dfa.h" //
+#include "tree-ssa.h"
+#include "tree-cfg.h"
+#include "tree-object-size.h" //
+#include "calls.h" //
+#include "cfgloop.h" //
+#include "intl.h"
+#include "gimple-range.h"
+
+#include "value-range.h"
+#include "gimple-range-path.h"
+#include "gcc-rich-location.h"
+#include "langhooks.h"
+#pragma GCC optimize("O0")
+namespace {
+
+const pass_data pass_data_wtype_demotion = {
+  GIMPLE_PASS,
+  "wtype_demotion",
+  OPTGROUP_NONE

[PATCH 0/5] function result decl location; type demotion

[Bernhard Reutner-Fischer via Fortran]

   return 1;
return 0;
}
int abc (int param1, int param2, int param3)
{
if (param1 == 42)
return 0;
if (param2 == 17)
return 0;
if (param3 == 99)
return 0;
return 0;
}
const void *const * pointer_thingie (void *i)
{
return (const void *const *)((long)i & 1);
}
int comparer (int param1, int param2, int param3) /* signed char */
{
if (param1 >= 42)
return 1;
if (param2 == 17)
return 1;
if (param3 <= 99)
return -1;
return 0;
}
int
zero_to_two (int i) /* suggest: unsigned char */
{
  if (i < 0)
return 0;
  if (i > 0)
return 2;
  return 0;
}
int main (void) // dg-bogus "Function .main. could return .bool."
{
return 0;
}


_Pragma("GCC visibility push(hidden)")
int my_main (int argc, char**argv) __attribute__(( visibility("default") ));
int my_main (int, char**) { return 0; }
int hidden_main (int, char**) { return 1; }
_Pragma("GCC visibility pop")


int i64c(int i)
{ /* returns [46,122] */
i &= 0x3f;
if (i == 0)
return '.';
if (i == 1)
return '/';
if (i < 12)
return ('0' - 2 + i);
if (i < 38)
return ('A' - 12 + i);
return ('a' - 38 + i);
}


EOF

Which gives for the normal C-flavour hunks:

$ ../gcc/xg++ -B../gcc -c -o /tmp/foo.o return-narrow.cc -O1 -Wall
return-narrow.cc: In function ‘int xyz(int, int, int)’:
return-narrow.cc:1:1: warning: Function ‘xyz’ could return ‘bool’ 
[-Wtype-demotion]
1 | int xyz (int param1, int param2, int param3)
  | ^~~
  | bool
return-narrow.cc:1:1: note: with a range of [0,1]
return-narrow.cc: In function ‘int abc(int, int, int)’:
return-narrow.cc:11:1: warning: Function ‘abc’ could return ‘bool’ 
[-Wtype-demotion]
   11 | int abc (int param1, int param2, int param3)
  | ^~~
  | bool
return-narrow.cc:11:1: note: with a range of [0,0]
return-narrow.cc: In function ‘const void* const* pointer_thingie(void*)’:
return-narrow.cc:21:7: warning: Function ‘pointer_thingie’ could return ‘bool’ 
[-Wtype-demotion]
   21 | const void *const * pointer_thingie (void *i)
  |   ^~~~
  |   bool
return-narrow.cc:21:7: note: with a range of [0B,1B]
return-narrow.cc: In function ‘int comparer(int, int, int)’:
return-narrow.cc:25:1: warning: Function ‘comparer’ could return ‘signed char’ 
[-Wtype-demotion]
   25 | int comparer (int param1, int param2, int param3) /* signed char */
  | ^~~
  | signed char
return-narrow.cc:25:1: note: with a range of [-1,1]
return-narrow.cc: In function ‘int zero_to_two(int)’:
return-narrow.cc:35:1: warning: Function ‘zero_to_two’ could return ‘unsigned 
char’ [-Wtype-demotion]
   35 | int
  | ^~~
  | unsigned char
return-narrow.cc:35:1: note: with a range of [0,2]
return-narrow.cc: In function ‘int my_main(int, char**)’:
return-narrow.cc:51:1: warning: Function ‘my_main’ could return ‘bool’ 
[-Wtype-demotion]
   51 | int my_main (int, char**) { return 0; }
  | ^~~
  | bool
return-narrow.cc:51:1: note: with a range of [0,0]
return-narrow.cc: In function ‘int hidden_main(int, char**)’:
return-narrow.cc:52:1: warning: Function ‘hidden_main’ could return ‘bool’ 
[-Wtype-demotion]
   52 | int hidden_main (int, char**) { return 1; }
  | ^~~
  | bool
return-narrow.cc:52:1: note: with a range of [1,1]
return-narrow.cc: In function ‘int i64c(int)’:
return-narrow.cc:55:1: warning: Function ‘i64c’ could return ‘unsigned char’ 
[-Wtype-demotion]
   55 | int i64c(int i)
  | ^~~
  | unsigned char
return-narrow.cc:55:1: note: with a range of [46,122]

So, any help wrt the template case? -- many TIA!


Bernhard Reutner-Fischer (5):
  c: Set the locus of the function result decl
  c++: Set the locus of the function result decl
  Fortran: Narrow return types [PR78798]
  value-range: Add as_string diagnostics helper
  gimple: Add pass to note possible type demotions; IPA pro/demotion

 gcc/Makefile.in  |   1 +
 gcc/c-family/c.opt   |   6 +
 gcc/c/c-decl.cc  |   6 +-
 gcc/cp/decl.cc   |  15 +-
 gcc/fortran/arith.cc |   4 +-
 gcc/fortran/arith.h  |   4 +-
 gcc/fortran/array.cc |   8 +-
 gcc/fortran/check.cc |   2 +-
 gcc/fortran/cpp.cc   |   3 +-
 gcc/fortran/cpp.h|   2 +-
 gcc/fortran/dependency.cc|   8 +-
 gcc/fortran/dependency.h |   6 +-
 gcc/fortran/expr.cc  |   6 +-
 gcc/fortran/gfortran.h   |  51 ++--
 gcc/fortran/intrinsic.cc |   6 +-
 gcc/fortran/io.cc|  13 +-
 gcc/fortran/misc.cc  |   2 +-
 gcc/fortran/parse.cc |   2 +-
 gcc/fortran/parse.h  |   2 +-
 gcc/fortran/primary.cc   |   4 +-
 gcc/fortran/resolve.cc   |  22 +-
 gcc/fortran/scanner.cc   |  20 +-
 gcc/fortran/symbol.cc   

[PATCH 1/5] c: Set the locus of the function result decl

[Bernhard Reutner-Fischer via Fortran]

Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
Ok for trunk?

Cc: Joseph Myers 
---
gcc/c/ChangeLog:

* c-decl.cc (start_function): Set the result decl source
location to the location of the typespec.
---
 gcc/c/c-decl.cc | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index a99b7456055..5250cb96c41 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -9980,6 +9980,7 @@ start_function (struct c_declspecs *declspecs, struct 
c_declarator *declarator,
   tree decl1, old_decl;
   tree restype, resdecl;
   location_t loc;
+  location_t result_loc;
 
   current_function_returns_value = 0;  /* Assume, until we see it does.  */
   current_function_returns_null = 0;
@@ -10206,8 +10207,11 @@ start_function (struct c_declspecs *declspecs, struct 
c_declarator *declarator,
   push_scope ();
   declare_parm_level ();
 
+  /* Set the result decl source location to the location of the typespec.  */
+  result_loc = (declspecs->locations[cdw_typespec] == UNKNOWN_LOCATION
+   ? loc : declspecs->locations[cdw_typespec]);
   restype = TREE_TYPE (TREE_TYPE (current_function_decl));
-  resdecl = build_decl (loc, RESULT_DECL, NULL_TREE, restype);
+  resdecl = build_decl (result_loc, RESULT_DECL, NULL_TREE, restype);
   DECL_ARTIFICIAL (resdecl) = 1;
   DECL_IGNORED_P (resdecl) = 1;
   DECL_RESULT (current_function_decl) = resdecl;
-- 
2.38.1

[PATCH 2/5] c++: Set the locus of the function result decl

[Bernhard Reutner-Fischer via Fortran]

gcc/cp/ChangeLog:

* decl.cc (start_function): Set the result decl source location to
the location of the typespec.

---
Bootstrapped and regtested on x86_86-unknown-linux with no regressions.
Ok for trunk?

Cc: Nathan Sidwell 
Cc: Jason Merrill 
---
 gcc/cp/decl.cc | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 6e98ea35a39..ed40815e645 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -17449,6 +17449,8 @@ start_function (cp_decl_specifier_seq *declspecs,
tree attrs)
 {
   tree decl1;
+  tree result;
+  bool ret;
 
   decl1 = grokdeclarator (declarator, declspecs, FUNCDEF, 1, );
   invoke_plugin_callbacks (PLUGIN_START_PARSE_FUNCTION, decl1);
@@ -17461,7 +17463,18 @@ start_function (cp_decl_specifier_seq *declspecs,
 gcc_assert (same_type_p (TREE_TYPE (TREE_TYPE (decl1)),
 integer_type_node));
 
-  return start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT);
+  ret = start_preparsed_function (decl1, attrs, /*flags=*/SF_DEFAULT);
+
+  /* decl1 might be ggc_freed here.  */
+  decl1 = current_function_decl;
+
+  /* Set the result decl source location to the location of the typespec.  */
+  if (TREE_CODE (decl1) == FUNCTION_DECL
+  && declspecs->locations[ds_type_spec] != UNKNOWN_LOCATION
+  && (result = DECL_RESULT (decl1)) != NULL_TREE
+  && DECL_SOURCE_LOCATION (result) == input_location)
+DECL_SOURCE_LOCATION (result) = declspecs->locations[ds_type_spec];
+  return ret;
 }
 
 /* Returns true iff an EH_SPEC_BLOCK should be created in the body of
-- 
2.38.1

[PATCH] Fortran: Remove unused declaration

[Bernhard Reutner-Fischer via Fortran]

This function definition was removed years ago, remove it's prototype.

gcc/fortran/ChangeLog:

* gfortran.h (gfc_check_include): Remove declaration.
---
 gcc/fortran/gfortran.h | 1 -
 1 file changed, 1 deletion(-)
---
Regtests cleanly, ok for trunk?

diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index c4deec0d5b8..ce3ad61bb52 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -3208,7 +3208,6 @@ int gfc_at_eof (void);
 int gfc_at_bol (void);
 int gfc_at_eol (void);
 void gfc_advance_line (void);
-int gfc_check_include (void);
 int gfc_define_undef_line (void);
 
 int gfc_wide_is_printable (gfc_char_t);
-- 
2.38.1

Re: adding attributes

[Bernhard Reutner-Fischer via Fortran]

Hi!

On Mon, 07 Nov 2022 11:04:17 +
Dave Love via Fortran  wrote:

> Bernhard Reutner-Fischer via Fortran  writes:
> 
> > I see.
> > So target_clones is one thing. What other attributes would be important?  
> 
> At least optimization-related ones could be useful, and possibly others.
> I haven't made a list, but could do.

Please do.
And yes, i can see that __attribute__((__optimize__(...))) would be
useful.

> dynamic dispatch in libraries.  (The worst thing about gfortran for
> system management is the lack of backwards-compatibility in module
> formats and libgfortran.)

yea. IIRC there was discussion a couple of years back what we could do
about the module format. Nowadays i'd probably just use JSON, but i did
not think too much about it. Needless to say that rewriting the mio
(module I/O) would take more than one evening :) I remember some
wrinkles there when i played around with the fortran-fe-stringpool idea
(which reminds me i should pickup again, maybe).

> > But since you cannot mix target_clones across arch-boundaries,
> > supporting those for a distro will probably be rather ugly anyway.  
> 
> Yes, you need simple pre-processing, as you do for the attributes in C,
> unless there was some extra guard facility added.

Yes indeed. But this would be much easier to handle if we'd have actual
arch defines. Until we have, you'd have to run this through cpp
manually which is doable but not all that convenient IMHO.

Hm, didn't we have a syntax for arch conditions in the math vec?
We could hijack that, but it's probably still better to just fix the
arch defines as that's generally useful.

> > heh, me neither. Luckily yesterday was a holiday, so what i ended up
> > with was the following, fya.  
> 
> Gosh; I thought it would take a while even if you knew your way around.
> I didn't want to spoil a holiday!  (I'd aim to do such things on work
> time.)

No problem, it was just for fun.
I spent most of the time to scratch my head why the attribute didn't
work for i had wrapped it in an arch ifdef for the testsuite to cover
both i386 and ppc. And of course i only noticed very, very late what was
really going on ;)

> > I've added a
> >/* Attributes set by compiler extensions (!GCC$ ATTRIBUTES).  */
> >unsigned ext_attr:EXT_ATTR_NUM;
> > +  tree ext_attr_args;
> >
> > to struct symbol_attribute where i can prepare the tree_list for the
> > attrs right from the start. The lowering is then rather simple and
> > uniform, just chainon the prepared attributes and be done.  
> 
> If I understand correctly, I could go through and add ones that look
> useful (for debate).  I have experience of using several in C (at least
> once even for g77 runtime).

Yes please, that'd be interesting.

> > target_clones does not require a bump in the module format, i'd say,
> > because the main entry point does not change. Will have to check if
> > the clones do not end up being emitted in the module, they shouldn't be.
> > Other attributes _may_ require a change in the module format though.
> > These would need checking on a per case basis.  
> 
> I don't understand the module format, but I wouldn't have expected
> relevant attributes to change interfaces.

Well that should probably not be needed indeed for most attributes, yes.

But then, i do think we stream out the ext_attr, at least for certain
attributes like "cdecl", the dll{im,ex}port, {std,fast}call et al.
See module.cc, mio_symbol_attribute. So the bits that change the
calling convention have to be brought to the attention of the module
consumer probably. Think regparm or sseregparm for example i guess.

That said, if i comment out the invalid cases of the test, i get
with gcc-12:
$ gfortran -c -o /tmp/out0.o 
/scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90 
/scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90:33:15:

   33 |   cdecl => sub2
  |   1
Warning: ‘cdecl’ attribute ignored [-Wattributes]
/scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90:33:15:
 Warning: ‘cdecl’ attribute ignored [-Wattributes]
/scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90:34:17:

   34 |   stdcall => sub3
  | 1
Warning: ‘stdcall’ attribute ignored [-Wattributes]
/scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90:34:17:
 Warning: ‘stdcall’ attribute ignored [-Wattributes]
/scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90:35:18:

   35 |   fastcall => sub4
  |  1
Warning: ‘fastcall’ attribute ignored [-Wattributes]
/scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/compiler-directive_1.f90:35:18:
 Warning: ‘fastcall’ attribute ignored [-Wattributes]

so i'm not sure what th

[PATCH 2/2] Fortran: Add attribute flatten

[Bernhard Reutner-Fischer via Fortran]

Bootstrapped and regtested cleanly on x86_unknown-linux.
The document bits will be rewritten for rst.
Ok for trunk if the prerequisite target_clones patch is approved?

gcc/fortran/ChangeLog:

* decl.cc (gfc_match_gcc_attributes): Handle flatten.
* f95-lang.cc (gfc_attribute_table): Add flatten.
* gfortran.texi: Document attribute flatten.

gcc/testsuite/ChangeLog:

* gfortran.dg/attr_flatten-1.f90: New test.
---
 gcc/fortran/decl.cc  |  8 +++-
 gcc/fortran/f95-lang.cc  |  2 +
 gcc/fortran/gfortran.texi|  8 
 gcc/testsuite/gfortran.dg/attr_flatten-1.f90 | 41 
 4 files changed, 57 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/attr_flatten-1.f90

diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc
index d312d4812b6..3d210c26eb5 100644
--- a/gcc/fortran/decl.cc
+++ b/gcc/fortran/decl.cc
@@ -11841,6 +11841,7 @@ gfc_match_gcc_attributes (void)
   for(;;)
 {
   char ch;
+  bool known_attr0args = false;
 
   if (gfc_match_name (name) != MATCH_YES)
return MATCH_ERROR;
@@ -11849,7 +11850,9 @@ gfc_match_gcc_attributes (void)
if (strcmp (name, ext_attr_list[id].name) == 0)
  break;
 
-  if (id == EXT_ATTR_LAST)
+  if (strcmp (name, "flatten") == 0)
+   known_attr0args = true; /* Handled below.  We do not need a bit.  */
+  else if (id == EXT_ATTR_LAST)
{
  gfc_error ("Unknown attribute in !GCC$ ATTRIBUTES statement at %C");
  return MATCH_ERROR;
@@ -11864,7 +11867,8 @@ gfc_match_gcc_attributes (void)
   || id == EXT_ATTR_DLLEXPORT
   || id == EXT_ATTR_CDECL
   || id == EXT_ATTR_STDCALL
-  || id == EXT_ATTR_FASTCALL)
+  || id == EXT_ATTR_FASTCALL
+  || known_attr0args)
attr.ext_attr_args
  = chainon (attr.ext_attr_args,
 build_tree_list (get_identifier (name), NULL_TREE));
diff --git a/gcc/fortran/f95-lang.cc b/gcc/fortran/f95-lang.cc
index 7154568aec5..ddb5b686cf6 100644
--- a/gcc/fortran/f95-lang.cc
+++ b/gcc/fortran/f95-lang.cc
@@ -101,6 +101,8 @@ static const struct attribute_spec gfc_attribute_table[] =
  gfc_handle_omp_declare_target_attribute, NULL },
   { "target_clones",  1, -1, true, false, false, false,
  gfc_handle_omp_declare_target_attribute, NULL },
+  { "flatten",0, 0, true,  false, false, false,
+ gfc_handle_omp_declare_target_attribute, NULL },
   { NULL,0, 0, false, false, false, false, NULL, NULL }
 };
 
diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi
index 06e4c8c00a1..be650f28b62 100644
--- a/gcc/fortran/gfortran.texi
+++ b/gcc/fortran/gfortran.texi
@@ -3280,6 +3280,14 @@ contains
 end module mymod
 @end smallexample
 
+@node flatten
+
+Procedures annotated with the @code{flatten} attribute have their
+callees inlined, if possible.
+Please refer to
+@ref{Top,,Common Function Attributes,gcc,Using the GNU Compiler Collection 
(GCC)}
+for details about the respective attribute.
+
 The attributes are specified using the syntax
 
 @code{!GCC$ ATTRIBUTES} @var{attribute-list} @code{::} @var{variable-list}
diff --git a/gcc/testsuite/gfortran.dg/attr_flatten-1.f90 
b/gcc/testsuite/gfortran.dg/attr_flatten-1.f90
new file mode 100644
index 000..0b72f1ba17c
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/attr_flatten-1.f90
@@ -0,0 +1,41 @@
+! { dg-do compile }
+! { dg-additional-options "-fdump-tree-optimized" }
+! Test __attribute__((flatten))
+!
+module attr_flttn_1_a
+  implicit none
+contains
+  subroutine sub1(i)
+integer, intent(in) :: i
+integer :: n
+do n = 1, i
+  print *, "marker1 ", i, i+n;
+enddo
+  end
+  subroutine sub2(i)
+integer, intent(in) :: i
+integer :: n
+do n = 1, i
+  print *, "marker2 ", i, i*i-n;
+enddo
+  end
+end module
+module attr_flttn_1_b
+  use attr_flttn_1_a
+contains
+  subroutine sub3
+!GCC$ ATTRIBUTES flatten :: sub3
+print *, "marker3 "
+call sub2(4711)
+call sub1(42)
+  end
+end module
+! Without the attribute flatten we would have 1 character write for each 
marker.
+! That would be 3 _gfortran_transfer_character_write.*marker
+! With the attribute, we have one for each sub plus marker1 and marker2
+! which were inlined into sub3.
+! So this gives 5 _gfortran_transfer_character_write.*marker
+! and there should be no calls to sub1 (); nor sub2 ();
+! { dg-final { scan-tree-dump-times { _gfortran_transfer_character_write 
.*?marker} 5 "optimized" } }
+! { dg-final { scan-tree-dump-not { sub1 \([^\)][^\)]*\);} "optimized" } }
+! { dg-final { scan-tree-dump-not { sub2 \([^\)][^\)]*\);} "optimized" } }
-- 
2.38.1

[PATCH 1/2] Fortran: Cleanup struct ext_attr_t

[Bernhard Reutner-Fischer via Fortran]

Tiny cleanup opportunity since we now have ext_attr_args in
struct symbol_attribute.
Bootstrapped and regtested on x86_64-unknown-linux with no new
regressions.
Ok for trunk if the prerequisite was approved ([PATCH 2/2] Fortran: add
attribute target_clones) ?

gcc/fortran/ChangeLog:

* gfortran.h (struct ext_attr_t): Remove middle_end_name.
* trans-decl.cc (add_attributes_to_decl): Move building
tree_list to ...
* decl.cc (gfc_match_gcc_attributes): ... here. Add the attribute to
the tree_list for the middle end.

Cc: gfortran ML 
---
 gcc/fortran/decl.cc   | 35 +++
 gcc/fortran/gfortran.h|  1 -
 gcc/fortran/trans-decl.cc | 13 +
 3 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc
index 3a619dbdd34..d312d4812b6 100644
--- a/gcc/fortran/decl.cc
+++ b/gcc/fortran/decl.cc
@@ -11802,15 +11802,15 @@ gfc_match_gcc_attribute_args (bool require_string, 
bool allow_multiple)
 }
 
 const ext_attr_t ext_attr_list[] = {
-  { "dllimport",EXT_ATTR_DLLIMPORT,"dllimport" },
-  { "dllexport",EXT_ATTR_DLLEXPORT,"dllexport" },
-  { "cdecl",EXT_ATTR_CDECL,"cdecl" },
-  { "stdcall",  EXT_ATTR_STDCALL,  "stdcall"   },
-  { "fastcall", EXT_ATTR_FASTCALL, "fastcall"  },
-  { "no_arg_check", EXT_ATTR_NO_ARG_CHECK, NULL},
-  { "deprecated",   EXT_ATTR_DEPRECATED,   NULL   },
-  { "target_clones",EXT_ATTR_TARGET_CLONES,NULL   },
-  { NULL,   EXT_ATTR_LAST, NULL}
+  { "dllimport",EXT_ATTR_DLLIMPORT },
+  { "dllexport",EXT_ATTR_DLLEXPORT },
+  { "cdecl",EXT_ATTR_CDECL },
+  { "stdcall",  EXT_ATTR_STDCALL   },
+  { "fastcall", EXT_ATTR_FASTCALL, },
+  { "no_arg_check", EXT_ATTR_NO_ARG_CHECK  },
+  { "deprecated",   EXT_ATTR_DEPRECATED},
+  { "target_clones",EXT_ATTR_TARGET_CLONES },
+  { NULL,   EXT_ATTR_LAST  }
 };
 
 /* Match a !GCC$ ATTRIBUTES statement of the form:
@@ -11854,6 +11854,20 @@ gfc_match_gcc_attributes (void)
  gfc_error ("Unknown attribute in !GCC$ ATTRIBUTES statement at %C");
  return MATCH_ERROR;
}
+
+  /* Check for errors.
+If everything is fine, add attributes the middle-end has to know about.
+   */
+  if (!gfc_add_ext_attribute (, (ext_attr_id_t)id, 
_current_locus))
+   return MATCH_ERROR;
+  else if (id == EXT_ATTR_DLLIMPORT
+  || id == EXT_ATTR_DLLEXPORT
+  || id == EXT_ATTR_CDECL
+  || id == EXT_ATTR_STDCALL
+  || id == EXT_ATTR_FASTCALL)
+   attr.ext_attr_args
+ = chainon (attr.ext_attr_args,
+build_tree_list (get_identifier (name), NULL_TREE));
   else if (id == EXT_ATTR_TARGET_CLONES)
{
  attr_args
@@ -11864,9 +11878,6 @@ gfc_match_gcc_attributes (void)
 build_tree_list (get_identifier (name), attr_args));
}
 
-  if (!gfc_add_ext_attribute (, (ext_attr_id_t)id, 
_current_locus))
-   return MATCH_ERROR;
-
   gfc_gobble_whitespace ();
   ch = gfc_next_ascii_char ();
   if (ch == ':')
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index ce0cb61e647..c4deec0d5b8 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -847,7 +847,6 @@ typedef struct
 {
   const char *name;
   unsigned id;
-  const char *middle_end_name;
 }
 ext_attr_t;
 
diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
index 24cbd4cda28..7d5d2bdbb37 100644
--- a/gcc/fortran/trans-decl.cc
+++ b/gcc/fortran/trans-decl.cc
@@ -1436,18 +1436,7 @@ gfc_add_assign_aux_vars (gfc_symbol * sym)
 static tree
 add_attributes_to_decl (symbol_attribute sym_attr, tree list)
 {
-  unsigned id;
-  tree attr;
-
-  for (id = 0; id < EXT_ATTR_NUM; id++)
-if (sym_attr.ext_attr & (1 << id) && ext_attr_list[id].middle_end_name)
-  {
-   attr = build_tree_list (
-get_identifier (ext_attr_list[id].middle_end_name),
-NULL_TREE);
-   list = chainon (list, attr);
-  }
-  /* Add attribute args.  */
+  /* Add attributes and their arguments.  */
   if (sym_attr.ext_attr_args != NULL_TREE)
 list = chainon (list, sym_attr.ext_attr_args);
 
-- 
2.38.1

[PATCH 0/2] Fortran: Add attribute flatten

[Bernhard Reutner-Fischer via Fortran]

Hi!

I could imagine that the flatten attribute might be useful.
Do we want to add support for it for gcc-13?

Bernhard Reutner-Fischer (2):
  Fortran: Cleanup struct ext_attr_t
  Fortran: Add attribute flatten

 gcc/fortran/decl.cc  | 41 +---
 gcc/fortran/f95-lang.cc  |  2 +
 gcc/fortran/gfortran.h   |  1 -
 gcc/fortran/gfortran.texi|  8 
 gcc/fortran/trans-decl.cc| 13 +--
 gcc/testsuite/gfortran.dg/attr_flatten-1.f90 | 41 
 6 files changed, 80 insertions(+), 26 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/attr_flatten-1.f90

-- 
2.38.1

[PATCH 0/2] Fortran: add attribute target_clones

[Bernhard Reutner-Fischer via Fortran]

Hi!

These two patches add support for attribute target_clones to the fortran
frontend.

1) The symtab hunk is identical to the one sent in
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/605081.html
which had some additional information.
I only tested it on x86_64-unknown-linux and for
languages c,fortran,c++,lto so far, i.e. not for ada,go,d,modula2

2) fortran FE hunks
I'm aware that the documentation will have to be rewritten now for
Sphinx.
I'll will take care of that before pushing the change anyway, of course.

The fortran FE part works as intended.
Note that it is of course not possible to mix ISA features of different
target architectures, i.e. "sse","avx512f" is fine, "powerpc7" in the
same attribute is prohibited.

There is a gotcha that took me a while to (re-)notice when writing the
testcase though: We (the fortran FE, fortran/cpp.cc) do not define arch
defines. Consider:
$ echo -e "#ifndef __x86_64__\nbummer\n#endif\nend" > /tmp/ouch.F95
$ ./gfortran -B. -c -o /tmp/ouch.o /tmp/ouch.F95
/tmp/ouch.F95:2:1:

2 | bummer
  | 1
Error: Unclassifiable statement at (1)

$ echo -e "#ifndef __x86_64__\nbummer\n#endif" \
  | ./xgcc -B. -x f95-cpp-input -ffree-form -E -dD - | grep -i x86
$ # nothing
versus
$ echo -e "#ifndef __x86_64__\nbummer\n#endif" \
  | ./xgcc -B. -x assembler-with-cpp -E -dD - | grep -i x86
#define __x86_64 1
#define __x86_64__ 1

I'm sure we have existing PRs about those missing defines but i didn't
look yet.


Bernhard Reutner-Fischer (2):
  symtab: also change RTL decl name
  Fortran: add attribute target_clones

 gcc/fortran/decl.cc   | 104 ++
 gcc/fortran/f95-lang.cc   |   4 +
 gcc/fortran/gfortran.h|   2 +
 gcc/fortran/gfortran.texi |  31 ++
 gcc/fortran/trans-decl.cc |   3 +
 gcc/symtab.cc |   6 +-
 .../gfortran.dg/attr_target_clones-1.F90  |  30 +
 7 files changed, 178 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/attr_target_clones-1.F90

-- 
2.38.1

[PATCH 1/2] symtab: also change RTL decl name

[Bernhard Reutner-Fischer via Fortran]

We were changing the ASSEMBLER_NAME of the function decl
but not the name in DECL_RTL which is used as the function name
fnname in rest_of_handle_final(). This led to using the old, wrong name
for the attribute target default function when using target_clones.

Bootstrapped and regtested cleanly on x86_64-unknown-linux
for c,c++,fortran,lto.
Ok for trunk?

gcc/ChangeLog:

* symtab.cc: Remove stray comment.
(symbol_table::change_decl_assembler_name): Also update the
name in DECL_RTL.

Cc: Jan Hubicka 
---
 gcc/symtab.cc | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/symtab.cc b/gcc/symtab.cc
index f2d96c0268b..2e20bf5fefc 100644
--- a/gcc/symtab.cc
+++ b/gcc/symtab.cc
@@ -154,8 +154,6 @@ symbol_table::decl_assembler_name_equal (tree decl, 
const_tree asmname)
 }
 
 
-/* Returns nonzero if P1 and P2 are equal.  */
-
 /* Insert NODE to assembler name hash.  */
 
 void
@@ -303,6 +301,10 @@ symbol_table::change_decl_assembler_name (tree decl, tree 
name)
warning (0, "%qD renamed after being referenced in assembly", decl);
 
   SET_DECL_ASSEMBLER_NAME (decl, name);
+  /* Set the new name in rtl.  */
+  if (DECL_RTL_SET_P (decl))
+   XSTR (XEXP (DECL_RTL (decl), 0), 0) = IDENTIFIER_POINTER (name);
+
   if (alias)
{
  IDENTIFIER_TRANSPARENT_ALIAS (name) = 1;
-- 
2.38.1

[PATCH 2/2] Fortran: add attribute target_clones

[Bernhard Reutner-Fischer via Fortran]

Hi!

Add support for attribute target_clones:
!GCC$ ATTRIBUTES target_clones("arch1", "arch3","default") :: mysubroutine

Bootstrapped and regtested on x86_64-unknown-linux with
--target_board=unix'{-m32,-m64}'.
OK for trunk?

gcc/fortran/ChangeLog:

* decl.cc: Include fold-const.h for size_int.
(gfc_match_gcc_attribute_args): New internal helper function.
(gfc_match_gcc_attributes): Handle target_clones.
* f95-lang.cc (struct attribute_spec): Add target and
target_clones entries.
* gfortran.h (ext_attr_id_t): Add EXT_ATTR_TARGET_CLONES.
(struct symbol_attribute): Add field ext_attr_args.
* trans-decl.cc (add_attributes_to_decl): Also add ext_attr_args
to the decl's attributes.
* gfortran.texi: Document attribute target_clones.

gcc/testsuite/ChangeLog:

* gfortran.dg/attr_target_clones-1.F90: New test.

Cc: gfortran ML 
---
 gcc/fortran/decl.cc   | 104 ++
 gcc/fortran/f95-lang.cc   |   4 +
 gcc/fortran/gfortran.h|   2 +
 gcc/fortran/gfortran.texi |  31 ++
 gcc/fortran/trans-decl.cc |   3 +
 .../gfortran.dg/attr_target_clones-1.F90  |  30 +
 6 files changed, 174 insertions(+)
 create mode 100644 gcc/testsuite/gfortran.dg/attr_target_clones-1.F90

diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc
index 0f9b2ced4c2..3a619dbdd34 100644
--- a/gcc/fortran/decl.cc
+++ b/gcc/fortran/decl.cc
@@ -23,6 +23,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "coretypes.h"
 #include "options.h"
 #include "tree.h"
+#include "fold-const.h"
 #include "gfortran.h"
 #include "stringpool.h"
 #include "match.h"
@@ -11709,6 +11710,96 @@ gfc_match_final_decl (void)
   return MATCH_YES;
 }
 
+/* Internal helper to parse attribute argument list.
+   If REQUIRE_STRING is true, then require a string.
+   If ALLOW_MULTIPLE is true, allow more than one arg.
+   If multiple arguments are passed, require braces around them.
+   Returns a tree_list of arguments or NULL_TREE.  */
+static tree
+gfc_match_gcc_attribute_args (bool require_string, bool allow_multiple)
+{
+  tree attr_args = NULL_TREE, attr_arg;
+  char name[GFC_MAX_SYMBOL_LEN + 1];
+  unsigned pos = 0;
+  gfc_char_t c;
+
+  /* When we get here, we already parsed
+ !GCC$ ATTRIBUTES ATTRIBUTE_NAME
+ Now parse the arguments. These could be one of
+   "single_string_literal"
+   ( "str_literal_1" , "str_literal_2" )
+   */
+
+  gfc_gobble_whitespace ();
+
+  if (allow_multiple && gfc_match_char ('(') != MATCH_YES)
+{
+  gfc_error ("expected '(' at %C");
+  return NULL_TREE;
+}
+
+  if (require_string)
+{
+  do {
+   if (pos)
+ {
+   if (!allow_multiple)
+ {
+   gfc_error ("surplus argument at %C");
+   return NULL_TREE;
+ }
+   gfc_next_ascii_char (); /* Consume the comma.  */
+ }
+   pos = 0;
+   gfc_gobble_whitespace ();
+   unsigned char num_quotes = 0;
+   do {
+ c = gfc_next_char_literal (NONSTRING);
+ if (c == '"')
+   {
+ num_quotes++;
+ continue; /* Skip the quote */
+   }
+ name[pos++] = c;
+ if (pos >= GFC_MAX_SYMBOL_LEN)
+   {
+ gfc_error ("attribute argument truncated at %C");
+ return NULL_TREE;
+   }
+   } while (num_quotes % 2 && gfc_match_eos () != MATCH_YES);
+   if (pos < 1)
+ {
+   gfc_error ("expected argument at %C");
+   return NULL_TREE;
+ }
+   if (num_quotes != 2)
+ {
+   gfc_error ("invalid string literal at %C");
+   return NULL_TREE;
+ }
+   name[pos] = '\0'; /* Redundant wrt build_string.  */
+   tree str = build_string (pos, name);
+   /* Compare with c-family/c-common.cc: fix_string_type.  */
+   tree i_type = build_index_type (size_int (pos));
+   tree a_type = build_array_type (char_type_node, i_type);
+   TREE_TYPE (str) = a_type;
+   TREE_READONLY (str) = 1;
+   TREE_STATIC (str) = 1;
+   attr_arg = build_tree_list (NULL_TREE, str);
+   attr_args = chainon (attr_args, attr_arg);
+
+   gfc_gobble_whitespace ();
+  } while (gfc_peek_ascii_char () == ',');
+}
+
+  if (allow_multiple && gfc_match_char (')') != MATCH_YES)
+{
+  gfc_error ("expected ')' at %C");
+  return NULL_TREE;
+}
+
+  return attr_args;
+}
 
 const ext_attr_t ext_attr_list[] = {
   { "dllimport",EXT_ATTR_DLLIMPORT,"dllimport" },
@@ -11718,6 +11809,7 @@ const ext_attr_t ext_attr_list[] = {
   { "fastcall", EXT_ATTR_FASTCALL, "fastcall"  },
   { "no_arg_check", EXT_ATTR_NO_ARG_CHECK, NULL},
   { "deprecated",   EXT_ATTR_DEPRECATED,   NULL   },
+  { "target_clones",EXT_ATTR_TARGET_CLONES,NULL   },
   { 

Re: [Patch] Fortran: Fix reallocation on assignment for kind=4 strings [PR107508]

[Bernhard Reutner-Fischer via Fortran]

On Sat, 5 Nov 2022 23:28:47 +0100
Tobias Burnus  wrote:

[no comment. I wonder why we malloc versus realloc in the first place,
i'd realloc always for starters. We end up calling into libc anyway, we
don't inline any of those calls and we suffer lib-boundary non-IPA
trouble either way, still, no? So why the conditional on our side?
valgrind certainly does not have a problem either way and i'd hope our
analyzer doesn't either. So what's that dance good for anyway? If you
did not remove it altogether anyway that is. ]

> PPS: I lost track of pending patches. Are they any which I should
> review?

You could poke honza in a while (which i will do if there is
interest) WRT
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/605081.html
needed iff we want to have (a cleaned-up version, including
gfortran.texi hunk) what David Love asked for in
https://gcc.gnu.org/pipermail/fortran/2022-October/058395.html
and where Thomas König expressed possible interest as per
https://gcc.gnu.org/pipermail/fortran/2022-November/058441.html
to add support for attribute target_clones.
I tested only c,c++,fortran,lto, don't have cycles for all langs, so if
you do d,ada,m2,..

The other 2 trivial hunks stemming from looking around that
attribute were approved already by Jeff and Richi, i'll push them
sometimes next week or during next weekend.

But David Love raised a question about more of the attributes supported
by the C FE, not just target_clones.

What are your thoughts around those?

blindvt> Tobias___, hi. You might have seen that David Love asked for
blindvt> attribute target_clones. Are there other attributes that you
blindvt> think would be helpful to support?
blindvt> i think flatten would be potentially usable, at least for
blindvt> power-users.
blindvt> not sure about simd. Maybe that? Or something
blindvt> else too? WDYT?
blindvt> i'm willing to spend one or two evenings on
blindvt> these, if you folks thing they are worth adding...

Apart from that, folks will certainly point at other patches pending
review..

thanks and cheers,

Re: adding attributes

[Bernhard Reutner-Fischer via Fortran]

On Sat, 5 Nov 2022 08:40:06 +0100
Thomas Koenig  wrote:

> On 04.11.22 21:59, Bernhard Reutner-Fischer via Fortran wrote:
> > And not sure if fellow gfortraners would accept this attribute
> > target_clones in there in the first place..  
> 
> It might actually be useful.  Is there any change about
> the calling sequence or anything else that should be visible
> in a Fortran module or the calling sequence?

The module interface remains the same.
And the call sequence remains the same, too.
For a user nothing changes.

An example:
module m
  implicit none
contains
  subroutine sub1()
!GCC$ ATTRIBUTES target_clones("avx", "sse","default") :: sub1
print *, 4321
  end
end module m

This used to compiles to:
$ nm /tmp/pristine.o 
 U _gfortran_st_write
 U _gfortran_st_write_done
 U _gfortran_transfer_integer_write
 T __m_MOD_sub1

And now compiles to:
$ nm /tmp/new.o 
 U __cpu_indicator_init
 U __cpu_model
 U _gfortran_st_write
 U _gfortran_st_write_done
 U _gfortran_transfer_integer_write
 i __m_MOD_sub1
006e t __m_MOD_sub1.avx
 t __m_MOD_sub1.default
 W __m_MOD_sub1.resolver
00dc t __m_MOD_sub1.sse

I.e. the caller still calls __m_MOD_sub1
But this is now an ifunc, which looks at the cpu bits and dispatches to
the appropriate ISA version.

I'm attaching the assembler input for reference.

If you think that we want to add support for that attribute, i can
submit a proper patch. Just let me know please.

thanks,
.file   "attr_target_clones-1.F90"
.text
.section.rodata
.align 8
.LC0:
.string 
"/scratch/src/gcc-13.mine/gcc/testsuite/gfortran.dg/attr_target_clones-1.F90"
.align 4
.LC1:
.long   4321
.text
.type   __m_MOD_sub1.default, @function
__m_MOD_sub1.default:
.LFB0:
.cfi_startproc
pushq   %rbp
.cfi_def_cfa_offset 16
.cfi_offset 6, -16
movq%rsp, %rbp
.cfi_def_cfa_register 6
subq$528, %rsp
movq$.LC0, -520(%rbp)
movl$21, -512(%rbp)
movl$128, -528(%rbp)
movl$6, -524(%rbp)
leaq-528(%rbp), %rax
movq%rax, %rdi
call_gfortran_st_write
leaq-528(%rbp), %rax
movl$4, %edx
movl$.LC1, %esi
movq%rax, %rdi
call_gfortran_transfer_integer_write
leaq-528(%rbp), %rax
movq%rax, %rdi
call_gfortran_st_write_done
nop
leave
.cfi_def_cfa 7, 8
ret
.cfi_endproc
.LFE0:
.size   __m_MOD_sub1.default, .-__m_MOD_sub1.default
.type   __m_MOD_sub1.avx, @function
__m_MOD_sub1.avx:
.LFB1:
.cfi_startproc
pushq   %rbp
.cfi_def_cfa_offset 16
.cfi_offset 6, -16
movq%rsp, %rbp
.cfi_def_cfa_register 6
subq$528, %rsp
movq$.LC0, -520(%rbp)
movl$21, -512(%rbp)
movl$128, -528(%rbp)
movl$6, -524(%rbp)
leaq-528(%rbp), %rax
movq%rax, %rdi
call_gfortran_st_write
leaq-528(%rbp), %rax
movl$4, %edx
movl$.LC1, %esi
movq%rax, %rdi
call_gfortran_transfer_integer_write
leaq-528(%rbp), %rax
movq%rax, %rdi
call_gfortran_st_write_done
nop
leave
.cfi_def_cfa 7, 8
ret
.cfi_endproc
.LFE1:
.size   __m_MOD_sub1.avx, .-__m_MOD_sub1.avx
.type   __m_MOD_sub1.sse, @function
__m_MOD_sub1.sse:
.LFB2:
.cfi_startproc
pushq   %rbp
.cfi_def_cfa_offset 16
.cfi_offset 6, -16
movq%rsp, %rbp
.cfi_def_cfa_register 6
subq$528, %rsp
movq$.LC0, -520(%rbp)
movl$21, -512(%rbp)
movl$128, -528(%rbp)
movl$6, -524(%rbp)
leaq-528(%rbp), %rax
movq%rax, %rdi
call_gfortran_st_write
leaq-528(%rbp), %rax
movl$4, %edx
movl$.LC1, %esi
movq%rax, %rdi
call_gfortran_transfer_integer_write
leaq-528(%rbp), %rax
movq%rax, %rdi
call_gfortran_st_write_done
nop
leave
.cfi_def_cfa 7, 8
ret
.cfi_endproc
.LFE2:
.size   __m_MOD_sub1.sse, .-__m_MOD_sub1.sse
.section
.text.__m_MOD_sub1.resolver,"axG",@progbits,__m_MOD_sub1.resolver,comdat
.weak   __m_MOD_sub1.resolver
.type   __m_MOD_sub1.resolver, @function
__m_MOD_sub1.resolver:
.LFB4:
.cfi_startproc
pushq   %rbp
.cfi_def_cfa_offset 16
.cf

Re: adding attributes

[Bernhard Reutner-Fischer via Fortran]

On Thu, 3 Nov 2022 00:19:26 +0100
Bernhard Reutner-Fischer  wrote:

> So target_clones is one thing. What other attributes would be important?

> > doing something previously!  (I don't know if I'll actually be able to
> > work on it in the end, at least on work time.)  
> 
> heh, me neither. Luckily yesterday was a holiday, so what i ended up
> with was the following, fya.
> Consider:

$ cat gcc/testsuite/gfortran.dg/attr_target_clones-1.F90; echo EOF
! { dg-require-ifunc "" }
! { dg-options "-O1" }
! { dg-additional-options "-fdump-tree-optimized" }
! It seems arch defines are not defined?!
! See fortran.cpp  FIXME: Pandora's Box
! Ok, so enterprise-level bugfix:
! { dg-additional-options "-D__i386__=1" { target { i?86-*-* x86_64-*-* } } }
! { dg-additional-options "-D__powerpc__=1" { target { powerpc*-*-* } } }
! { dg-skip-if "test not yet implemented for target" { ! {i?86-*-* x86_64-*-* 
powerpc*-*-*} } }
!! { dg- skip-if "needs optimize" { *-*-* } { "*" } { " -O0 " } }
! Test __attribute__ ((target_clones ("foo", "bar")))
!
module m
  implicit none
contains
  subroutine sub1()
#if defined __i386__ || defined __x86_64__
!GCC$ ATTRIBUTES target_clones("avx", "sse","default") :: sub1
#elif defined __powerpc__
!GCC$ ATTRIBUTES target_clones("power10", "power9","default") :: sub1
#endif
print *, 4321
  end
end module m
! { dg-final { scan-tree-dump-times {(?n)void \* __m_MOD_sub1\.resolver \(\)} 1 
"optimized" } }
! { dg-final { scan-tree-dump-times {(?n)void __m_MOD_sub1\.(?:avx|power10) 
\(\)} 1 "optimized" } }
! { dg-final { scan-tree-dump-times {(?n)void __m_MOD_sub1\.(?:sse|power9) 
\(\)} 1 "optimized" } }
! { dg-final { scan-tree-dump-times {(?n)void sub1 \(\)} 1 "optimized" } }
!! and a non-assembly hint on the ifunc
! { dg-final { scan-tree-dump-times {Function sub1 \(__m_MOD_sub1\.default,} 1 
"optimized" } }
EOF

2 patches:
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/605081.html
https://gcc.gnu.org/pipermail/gcc-patches/2022-November/604981.html
(the testcase mentioned in the latter is superseded be the blurb above)

One would have to cleanup the parser (see "XXX: Rephrase this in a
sane, understandable manner..") and add some more testcases, for several
malformed attribute strings. Maybe i'll get to it during the weekend or
some evening.

Not sure about the usefulness though.
And not sure if fellow gfortraners would accept this attribute
target_clones in there in the first place..

cheers,

Re: adding attributes

[Bernhard Reutner-Fischer via Fortran]

On Fri, 28 Oct 2022 15:35:45 +0100
Dave Love via Fortran  wrote:

> Assuming there's no technical reason not to, can someone say what would
> be involved in adding relevant attributes (at least function ones) like
> those for C?  I'm thinking particularly of target_clones, but others
> probably make sense.

Well we already have
!GCC$ ATTRIBUTES attribute-list :: var-name [, var-name] ...

See https://gcc.gnu.org/onlinedocs/gfortran/ATTRIBUTES-directive.html

> 
> I don't know my way around, but I had a quick look, and it at least
> wouldn't be as straightforward as I hoped.

See gcc/fortran/decl.cc gfc_match_gcc_* 

For example the CDECL attribute probably comes close to target_clones:
subroutine mysub1()
!GCC$ ATTRIBUTES CDECL :: mysub1
! body of mysub1 here

For target_clones you would most likely need a slightly different parser
for you need the user to specify the actual target_clones somehow. You
would probably make a suggestion and discuss the proposal here.
Ideally the syntax would be the same as in C.

Finally you would have to lower the target_clones, not sure offhand who
creates the ifunc dispatcher, the frontend or the middle-end. There is
expand_target_clones in gcc/multiple_target.cc that probably comes into play.
So yes, that seems to create the ifuncs if i read that correctly.
Hence the implementation in the frontend should not be all too
complicated, it seems.

To your initial remark about a technical reason not to support it i
would point you to what Tobias said to me some time ago and which i
certainly told other people more than once, too:
https://patchwork.ozlabs.org/comment/958570/
---8<---
In general, I prefer to stick to standard methods
(which are portable) and think that those user knobs often make things
slower than faster (as they tend to stay for years, even after the hard-
ware as moved on - or they are even inserted blindly).
---8<---

In former times, you would compile your library multiple times
and provide a distinct, optimized version for each of the CPUs.
Maybe that would work for you equally well, without target_clones?

HTH

Re: Adding a new thread model to GCC

[Bernhard Reutner-Fischer via Fortran]

On 4 October 2022 10:06:00 CEST, LIU Hao  wrote:
>在 2022-10-03 13:03, Bernhard Reutner-Fischer 写道:
>> 
>> No, sorry for my brevity.
>> Using __gthread_t like in your patch is correct.
>> 
>
>I see. In 'libgfortran/io/async.c' there is
>
>  ```
>async_unit *au = u->au;
>LOCK (>lock);
>thread_unit = u;
>au->thread = __gthread_self ();
>  ```
>
>so indeed `thread` should be `__gthread_t`.

Yes.

> By the way I reported this issue four months ago and haven't received any 
> response so far:
>
>  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105764

So, ideally, you would mention this PR in your patch.

LGTM (obvious even) but I cannot formally approve it.
thanks,

Re: Adding a new thread model to GCC

[Bernhard Reutner-Fischer via Fortran]

On 2 October 2022 14:54:54 CEST, LIU Hao  wrote:
>在 2022-10-02 04:02, Bernhard Reutner-Fischer 写道:
>> On 1 October 2022 20:34:45 CEST, LIU Hao via Gcc-patches 
>>  wrote:
>>> Greetings.
>> 
>>> The first patch is necessary because somewhere in libgfortran, `pthread_t` 
>>> is referenced. If the thread model is not `posix`, it fails to compile.
>> 
>> One of several shortcomings mentioned already on Sun, 02 Sep 2018 15:40:28 
>> -0700 in
>> https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg196212.html
>> 
>
>Forgive me but I didn't get your point. Is the 'shortcoming' the fact that 
>`pthread_t` must be preferred to `__gthread_t`?

No, sorry for my brevity.
Using __gthread_t like in your patch is correct.

thanks,

>
>For non-posix thread models,  is not included, so `pthread_t` is 
>not declared. I haven't looked at other code in libgfortran, but changing 
>`pthread_t` to `__gthread_t` does allow libgfortran to build. I don't know how 
>to test it though, as I don't write Fortran myself.
>
>

Re: Adding a new thread model to GCC

[Bernhard Reutner-Fischer via Fortran]

On 1 October 2022 20:34:45 CEST, LIU Hao via Gcc-patches 
 wrote:
>Greetings.

>The first patch is necessary because somewhere in libgfortran, `pthread_t` is 
>referenced. If the thread model is not `posix`, it fails to compile.

One of several shortcomings mentioned already on Sun, 02 Sep 2018 15:40:28 
-0700 in
https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg196212.html

Re: [PATCH 09/10] fortran: Support clobbering of variable subreferences [PR88364]

[Bernhard Reutner-Fischer via Fortran]

On 17 September 2022 21:50:20 CEST, Mikael Morin  wrote:
>Le 17/09/2022 à 21:33, Mikael Morin a écrit :
>> The testcase from the patch was not specifically checking lack of 
>> side-effect clobbers, so I have double-checked with the following testcase, 
>> which should lift your concerns.
>> 
>The dump matches didn’t fail as expected with patch 2/10 reversed.
>This testcase should be better.

! { dg-final { scan-tree-dump-times "456" 0 "optimized" { target __OPTIMIZE__ } 
} }

I'd spell this as scan-tree-dump-not, fwiw.

That said, plain scan-tree-dump is usually only viable in arch influenced 
checks which in fortran we do not usually have. Here, we should for the most 
part use -not or a specific -times.

I think you had a check for integer(kind=4) in there, too, which might not work 
all that well for -fdefault-integer-8 or, for the corresponding real scan, 
-fdefault-real-8, eventually. Easily tweaked on top if anyone (certainly will) 
complain later on, though..

fore, either way, I'd say :-)
thanks,

Re: [PATCH 09/10] fortran: Support clobbering of variable subreferences [PR88364]

[Bernhard Reutner-Fischer via Fortran]

On 17 September 2022 21:33:22 CEST, Mikael Morin  wrote:
>Le 17/09/2022 à 19:03, Thomas Koenig via Fortran a écrit :
>> 
>> Hi Mikael,
>> 
>>> This adds support for clobbering of partial variable references, when
>>> they are passed as actual argument and the associated dummy has the
>>> INTENT(OUT) attribute.
>>> Support includes array elements, derived type component references,
>>> and complex real or imaginary parts.
>>> 
>>> This is done by removing the check for lack of subreferences, which is
>>> basically a revert of r9-4911-gbd810d637041dba49a5aca3d085504575374ac6f.
>>> This removal allows more expressions than just array elements,
>>> components and complex parts, but the other expressions are excluded by
>>> other conditions: substrings are excluded by the check on expression
>>> type (CHARACTER is excluded), KIND and LEN references are rejected by
>>> the compiler as not valid in a variable definition context.
>>> 
>>> The check for scalarness is also updated as it was only valid when there
>>> was no subreference.
>> 
>> First, thanks a lot for digging into this subject. I have looked through
>> the patch series, and it looks very good so far.

I second that!
The series looks plausible IMO.

>> 
>> I have a concern about this part, though.  My understanding at the
>> time was that it is not possible to clobber an individual array
>> element, but that this clobbers anything off the pointer that this
>> is based on.
>> 
>Well, we need the middle-end guys to give a definitive answer on this topic, 
>but I think it would be a very penalizing limitation if that was the case.  I 
>have assumed that the clobber spanned the value it was applied on, neither 
>more nor less, so just the array element in case of array elements.

I would assume the same, fwiw.
Let's blame the ME iff something goes amiss then, but I doubt it will.

>> So,
>> 
>>    integer, dimension(3) :: a
>> 
>>    a(1) = 1
>>    a(3) = 3
>>    call foo(a(1))
>> 
>> would also invalidate the store to a(3).  Is my understanding correct?
>
>I think it was the case before patch 2 in in the series, because the clobber 
>was applied to the symbol decl, so in the case of the expression A(1), it was 
>applied to A which is the full array.  After patch 2, the clobber is applied 
>to the expression A(1), so the element alone.

Yep.

>> If so, I think this we cannot revert that patch (which was introduced
>> because of a regression).
>> 
>The testcase from the patch was not specifically checking lack of side-effect 
>clobbers, so I have double-checked with the following testcase, which should 
>lift your concerns.
>I propose to keep the patch with the testcase added to it.  What do you think?

I cannot approve it but the series looks good to me.

Thanks!

Re: [PATCH] Fortran: add IEEE_QUIET_* and IEEE_SIGNALING_* comparisons

[Bernhard Reutner-Fischer via Fortran]

On 2 September 2022 17:54:00 CEST, FX  wrote:
>Hi Bernhard,
>
>> Please do not call the non-standard abort, but use stop N.
>
>Is there a specific reason? It’s a well-documented GNU extension, and it’s 
>useful because it can easily display a backtrace and give line info for the 
>failure, unlike STOP.
>I’ll replace if there is consensus, but apart from aesthetics I don’t see why.


IIRC there was discussion about abort on the ML some years ago where folks 
decided to switch to stop N.
I don't think I participated in that discussion, maybe somebody remembers the 
reasoning or is able to find the thread.

thanks,

Re: [PATCH] Fortran: add IEEE_QUIET_* and IEEE_SIGNALING_* comparisons

[Bernhard Reutner-Fischer via Fortran]

On 2 September 2022 13:37:41 CEST, FX via Fortran  wrote:
>Hi,

Please do not call the non-standard abort, but use stop N.

IIRC I once had a trivial script.. 
https://www.mail-archive.com/search?l=gcc-patc...@gcc.gnu.org=subject:%22%5C%5BPATCH%2C+OpenACC%5C%5D+Fortran+deviceptr%22=newest=1

---8<---
Like (modulo typos, untested):
$ cat abort_to_stop.awk ; echo EOF
# awk -f ./abort_to_stop.awk < foo.f90 > x && mv x foo.f90
BEGIN { IGNORECASE = 1; i = 1 } { while (sub(/call\s\s*abort/, "stop " i)) {let 
i++;}; print $0; }
EOF

HTH and thanks,

Re: [PATCH] Fortran 2018 rounding modes changes

[Bernhard Reutner-Fischer via Fortran]

On 31 August 2022 20:29:12 CEST, FX via Fortran  wrote:


+  case GFC_FPE_GFC_FPE_AWAY:

typo?

thanks,

Re: [RFC] fortran: restore array indexing for all descriptor arrays [PR102043]

[Bernhard Reutner-Fischer via Fortran]

On 2 July 2022 14:47:01 CEST, Mikael Morin  wrote:
>Le 02/07/2022 à 13:18, Thomas Koenig a écrit :
>> 
>> One thought: If we have to bite the bullet and break the ABI, why not go
>> all the way and use the C descriptors in ISO_Fortran_binding.h as
>> gfortran's native format?
>> 
>As far as I know, CFI descriptors are mutually exclusive with
>non-negative array indexes.
>
>CFI descriptors' base_addr field points to the first element in
>descriptor indexing order, so they can be indexed negatively and we
>can’t safely use array indexing with them.
>
>With an additional offset field it would be possible though (with a
>different semantics for the base_addr field).  Or we just keep the
>different semantics of the base_addr field and assume that there is an
>implicit offset with value sum(extent * max(-sm, 0)).
>
>Anyway, this is the long-delayed array descriptor reform, right?  Any

I think so, yes.

>one remembers how far we are from it?

We have a wiki page with notes IIRC.

Re: [PATCH 6/8] fortran: use grep -F instead of fgrep

[Bernhard Reutner-Fischer via Fortran]

On 24 June 2022 14:35:20 CEST, Rainer Orth  
wrote:
>Hi Xi,
>
>> On Fri, 2022-06-24 at 13:13 +0200, Bernhard Reutner-Fischer wrote:
>>
>>> > -   if $(SHELL) -c 'install-info --version | sed 1q | fgrep -s -v
>>> > -i debian' >/dev/null 2>&1; then \
>>> > +   if $(SHELL) -c 'install-info --version | sed 1q | grep -F -s -v
>>> > -i debian' >/dev/null 2>&1; then \
>>> >   echo " install-info --delete --info-dir=$(DESTDIR)$(infodir)
>>> > $(DESTDIR)$(infodir)/gfortran.info"; \
>>> >   install-info --delete --info-dir=$(DESTDIR)$(infodir)
>>> > $(DESTDIR)$(infodir)/gfortran.info || : ; \
>>> > else : ; fi; \
>>> 
>>> I'd replace -s >/dev/null 2>&1 with -q while at it.
>>> 
>>> But why is -F used here in the first place?
>>> I do not see much in debian that can be interpreted as a regex?
>>
>> I'm not sure.  It was there since 2004.  Perhaps the author thinks fgrep
>> may save several CPU cycles :).  I'll just use a plain grep in PATCH v2.
>>
>> Rainer: do you have some idea about the availability of "-q" on
>> different hosts?  If you agree I'll use it instead of -s > /dev/null
>> too.
>
>again, the autoconf manual warns agains it, even more so against -s.
>That's the first reference for portability issues and shouldn't be
>ignored without good reason.
>
>In the GCC and Solaris context, /bin/grep supports both -q and -s in
>Solaris 11.3 and 11.4.  It doesn't support -q on Solaris 10, though
>(again, no longer relevant for GCC).

The good reason I would bring forward is that the systems mentioned in the 
autoconf docs are all not supported anymore (IRIX, ancient or old Solaris etc).
Furthermore, grep(1) is required by POSIX to support -q since at least 1997; 
see https://pubs.opengroup.org/onlinepubs/007908799/xcu/grep.html

That's about 25 years now, so everybody had plenty of time to implement this 
specific part, which is even trivial to implement for this particular case of 
grep -q.

All in all i think that we should not be held hostage by such systems any 
longer, at least for such cases that are trivial to fix to conformance. Of 
course that may be just /me.

Iff fixing egrep or fgrep occurrences though,  we should use plain grep where 
applicable, like in this case, IMO.

thanks,

Re: [PATCH 6/8] fortran: use grep -F instead of fgrep

[Bernhard Reutner-Fischer via Fortran]

On Fri, 24 Jun 2022 15:06:32 +0800
Xi Ruoyao via Gcc-patches  wrote:

> fgrep has been deprecated in favor of grep -F for a long time, and the
> next grep release (3.8 or 4.0) will print a warning of fgrep is used.
> Stop using fgrep so we won't see the warning.
> 
> gcc/ChangeLog:
> 
>   * fortran/Make-lang.in: Use grep -F instead of fgrep.
> ---
>  gcc/fortran/Make-lang.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/fortran/Make-lang.in b/gcc/fortran/Make-lang.in
> index 1cb47cb1a52..51279b03aad 100644
> --- a/gcc/fortran/Make-lang.in
> +++ b/gcc/fortran/Make-lang.in
> @@ -278,7 +278,7 @@ $(DESTDIR)$(man1dir)/$(GFORTRAN_INSTALL_NAME)$(man1ext): 
> doc/gfortran.1 \
>   -chmod a-x $@
>  
>  fortran.uninstall:
> - if $(SHELL) -c 'install-info --version | sed 1q | fgrep -s -v -i 
> debian' >/dev/null 2>&1; then \
> + if $(SHELL) -c 'install-info --version | sed 1q | grep -F -s -v -i 
> debian' >/dev/null 2>&1; then \
> echo " install-info --delete --info-dir=$(DESTDIR)$(infodir) 
> $(DESTDIR)$(infodir)/gfortran.info"; \
> install-info --delete --info-dir=$(DESTDIR)$(infodir) 
> $(DESTDIR)$(infodir)/gfortran.info || : ; \
>   else : ; fi; \

I'd replace -s >/dev/null 2>&1 with -q while at it.

But why is -F used here in the first place?
I do not see much in debian that can be interpreted as a regex?

thanks,

Re: GSoC Blog Post 0 - GCCprefab build system

[Bernhard Reutner-Fischer via Fortran]

On 17 June 2022 21:55:47 CEST, Jakub Jelinek  wrote:
>On Fri, Jun 17, 2022 at 08:45:04PM +0200, Bernhard Reutner-Fischer via Gcc 
>wrote:
>> PS: we should rm 
>> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=contrib/gcc_build
>
>No.  gcc_build is used by maintainer-scripts/gcc_release, so by killing it
>you'd make gcc unreleasable.

Doh, didn't grep in maintainer-scripts.
So, should the SVN parts be ripped out?

For non-maintainers, switch it to git instead?

>> It was not updated since the switch to git so, apparently, nobody uses it 
>> anymore (sadly, seeing who authored it).

The hunks talking about subversion do seem to be misleading nowadays, aren't 
they?

thanks,

Re: GSoC Blog Post 0 - GCCprefab build system

[Bernhard Reutner-Fischer via Fortran]

On 13 June 2022 17:26:59 CEST, Jonathan Wakely via Fortran 
 wrote:

>https://gist.github.com/jwakely/95b3a790157f55d75e18f577e12b50d7#file-build_gcc_versions-sh

s/[[/[/
s/==/=/

The former are deprecated or obsolescent notations of test(1) syntax, fwiw.

PS: we should rm https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=contrib/gcc_build
It was not updated since the switch to git so, apparently, nobody uses it 
anymore (sadly, seeing who authored it).

Re: [patch, fortran, doc] Mention new CONVERT options for POWER

[Bernhard Reutner-Fischer via Fortran]

On Fri, 29 Apr 2022 20:03:55 +0200
Thomas Koenig  wrote:

> On 28.04.22 19:17, Bernhard Reutner-Fischer wrote:
> > ISTM that this should be s/mod.e/mode./ ?  
> 
> Yep, fixed as obvious and simple on trunk with
> r13-49-g4a8b45e8bc8246bd141dad65f571a3e0cc499c6b .

thanks!
> 
> OK for backport to gcc-12? (This is both extremely safe and
> not particularly important :-)

I'd say yes because it's 1) doc 2) trivial 3) obvious :)

But formally, i think RM approval is needed ATM.
Richard?

Re: [patch, fortran, doc] Mention new CONVERT options for POWER

[Bernhard Reutner-Fischer via Fortran]

Hi Thomas,

On 27 April 2022 22:34:39 CEST, Thomas Koenig via Fortran  
wrote:

+@item @code{'big_endian'}  Do all unformatted I/O in big_endian mod.e


ISTM that this should be s/mod.e/mode./ ?
thanks,

Re: [pushed] testsuite: add additional option to force DSE execution [PR103662]

[Bernhard Reutner-Fischer via Fortran]

On 25 April 2022 14:12:30 CEST, Jakub Jelinek via Fortran  
wrote:
>On Mon, Apr 25, 2022 at 01:38:25PM +0200, Mikael Morin wrote:
>> I have just pushed the attached fix for two UNRESOLVED checks at -O0 that I
>> hadn’t seen.
>
>I don't like forcing of DSE in -O0 compilation, wouldn't it be better
>to just not check the dse dump at -O0 like in the following patch?

As Mikael already said, that's preferable indeed.

>
>Even better would be to check that the z._data = stores are both present
>in *.optimized dump, but that doesn't really work at -O2 or above because
>we inline the functions and optimize it completely away (both the stores
>and corresponding reads).
>
>The first hunk is needed so that __OPTIMIZE__ effective target works in
>Fortran testsuite, otherwise one gets a pedantic error and __OPTIMIZE__
>is considered not to match at all.
>
>2022-04-25  Jakub Jelinek  
>
>   PR fortran/103662
>   * lib/target-supports.exp (check_effective_target___OPTIMIZE__): Add
>   a var definition to avoid pedwarn about empty translation unit.
>   * gfortran.dg/unlimited_polymorphic_3.f03: Remove -ftree-dse from
>   dg-additional-options, guard scan-tree-dump-not directives on
>   __OPTIMIZE__ target.
>
>--- gcc/testsuite/lib/target-supports.exp.jj   2022-04-22 13:36:56.150960826 
>+0200
>+++ gcc/testsuite/lib/target-supports.exp  2022-04-25 14:06:21.620537483 
>+0200
>@@ -11770,6 +11770,7 @@ proc check_effective_target___OPTIMIZE__
>   #ifndef __OPTIMIZE__
>   # error nein

Hmz. I _think_ that was /me and reading it again now, we should should not 
bluntly say nein but more something like
unwilling to ignore __OPTIMIZE__, dude

or anything more descriptive and universally understandably I assume.

>   #endif
>+  int optimize;

a plain ; won't cut it. int dummy, maybe though. There is probably a lot more 
of these, aren't there?

thanks,

> } [current_compiler_flags]]
> }
> 
>--- gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03.jj   2022-04-25 
>13:54:38.320309119 +0200
>+++ gcc/testsuite/gfortran.dg/unlimited_polymorphic_3.f03  2022-04-25 
>14:04:01.346486431 +0200
>@@ -1,5 +1,5 @@
> ! { dg-do run }
>-! { dg-additional-options "-ftree-dse -fdump-tree-dse-details" }
>+! { dg-additional-options "-fdump-tree-dse-details" }
> !
> ! Check that pointer assignments allowed by F2003:C717
> ! work and check null initialization of CLASS(*) pointers.
>@@ -71,5 +71,5 @@ end subroutine foo_sq
> ! We used to produce multiple independant types for the unlimited polymorphic
> ! descriptors (types for class(*)) which caused stores to them to be seen as
> ! useless.
>-! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = " "dse1" } 
>}
>-! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = " "dse1" } 
>}
>+! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = " "dse1" { 
>target __OPTIMIZE__ } } }
>+! { dg-final { scan-tree-dump-not "Deleted dead store: z._data = " "dse1" { 
>target __OPTIMIZE__ } } }
>
>
>   Jakub
>

Re: [PATCH] git-backport: support renamed .cc files in commit message.

[Bernhard Reutner-Fischer via Fortran]

On Wed, 12 Jan 2022 16:54:46 +0100
Martin Liška  wrote:

> +def replace_file_in_changelog(lines, filename):
> +if not filename.endswith('.cc'):
> +return
> +
> +# consider all componenets of a path: gcc/ipa-icf.cc
> +while filename:
> +for i, line in enumerate(lines):
> +if filename in line:
> +line = line.replace(filename, filename[:-1])
> +lines[i] = line
> +return
> +parts = filename.split('/')
> +if len(parts) == 1:
> +return
> +filename = '/'.join(parts[1:])
> +

I think you mean os.sep instead of the hardcoded slash.
But i'd use os.path.split and os.path.join

thanks,