[Fortran] Patch ping

[Tobias Burnus-2]

Dear all,

first, I would like to ping my patch:
- [Patch, Fortran] PR52864 - fix actual/formal checks
  http://gcc.gnu.org/ml/fortran/2012-04/msg00059.html

Other patches with pending review:
- [Patch, Fortran, F03] PR52909: Procedure pointers not private to modules
  http://gcc.gnu.org/ml/fortran/2012-04/msg00033.html
  Caveat: ABI breakage
- [patch, fortran] PR fortran/52537 Optimize string comparisons against empty strings
  http://gcc.gnu.org/ml/fortran/2012-04/msg00068.html
- [Patch, libfortran] Combine get_mem and internal_malloc_size
  http://gcc.gnu.org/ml/fortran/2012-03/msg00127.html


Approved but not yet committed:

My patches:
- (My "TREE_PUBLIC() = 0 for module procedures" test-case fix
   http://gcc.gnu.org/ml/fortran/2012-04/msg00082.html)
- [Patch, Fortran] PR52864 - Fix pointer-intent regresssion
  http://gcc.gnu.org/ml/fortran/2012-04/msg00058.html

Janus:
- [Patch, Fortran, OOP] PR 52968: Call to type-bound procedure wrongly rejected
  http://gcc.gnu.org/ml/fortran/2012-04/msg00083.html

Thomas:
- [patch, fortran] Trim spaces on list-directed reads
  http://gcc.gnu.org/ml/fortran/2012-04/msg00040.html
- [patch, fortran-dev] Use fixed variable sizes for stride calculations
  http://gcc.gnu.org/ml/fortran/2012-04/msg00074.html

Bernhard:
- [PATCH] gfortran testsuite: implicitly cleanup-modules, part 2
  http://gcc.gnu.org/ml/fortran/2012-04/msg00065.html

Janne:
- [Patch, fortran] PR 49010/24518 MOD/MODULO fixes
  http://gcc.gnu.org/ml/fortran/2012-04/msg00012.html
  Okayed but haven't found best wording.


 * * *

gfortran regression status:

- PR52916 - [4.8 Regression] 481.wrf in SPEC CPU 2006 failed to build
  Fixed - except for followup test-suite fix
- PR 52864 - [4.6/4.7/4.8 Regression] Assignment to pointer component ...
  rejects-valid. Approved patch, not yet committed.
  (Non regression issue: Patch submitted, pending review)
- PR 52679 - [4.6 Regression] ICE in gfortran 4.6.3, x86_64
  ice-on-valid-code.
- PR 45586 - [4.8 Regression] ICE non-trivial conversion at assignment
  ice-checking (tree decl issue related to "restrict"ed pointers.)
- PR49791 - [4.5/4.6/4.7/4.8 Regression] Formatted namelist reads fai...
  Special case remaining. Related to more serious namelist nonregression
  PR 51825
- PR50981 - [4.5/4.6 Regression] Wrong-code for scalarizing ELEMENTAL...
  Mostly fixed. Could consider skipping the backporting. Some non-regressions
  still have to be fixed.
- PR51520 - [4.6 Regression] ICE in gfortran 4.6.2, x86_64
  ice-on-valid-code
- PR52062 - [4.6 regression] public generic name, specific functions ...
  ice-on-valid-code
- PR50410 - [4.6/4.7/4.8 Regression] ICE in record_reference
  ice-on-invalid-code  Multiple data initialization, draft patch available
- PR50105. [4.6/4.7/4.8 Regression] I/O with g6.5 - wrong number of ...
  wrong-code. Not a regression (according to J3, WG5 approval pending);
  missed diagnostic
- PR 52158 - Regression on character function with gfortran 4.7
  No true regression. Bogusly rejects previously working character(len=:),
  but the support was shaky.
- PR 42954 - [4.5/4.6/4.7/4.8 regression] TARGET_*_CPP_BUILTINS issues...


Tobias

PS: I hope that I found all pending patches and correctly stated their status.

[Thomas Koenig-6]

Hi Tobias,

> - [patch, fortran] Trim spaces on list-directed reads
>    http://gcc.gnu.org/ml/fortran/2012-04/msg00040.html

That one was committed:
http://gcc.gnu.org/ml/gcc-cvs/2012-04/msg00417.html

Jerry indicated that this would also be OK for a backport; I'll
do that within a few days unless there are objections.

> - [patch, fortran-dev] Use fixed variable sizes for stride calculations
>    http://gcc.gnu.org/ml/fortran/2012-04/msg00074.html

That one is

http://gcc.gnu.org/ml/gcc-cvs/2012-04/msg00400.html

A more general question: I habe been a bit inconsistent in notifying
the mailing list about committed patches; I didn't do this for these
patches.

What would people, generally, prefer?  Should we notify on commit,
or rather not?

Regards

        Thomas

[Tobias Burnus]

Thomas Koenig wrote:
>> - [patch, fortran] Trim spaces on list-directed reads
>>    http://gcc.gnu.org/ml/fortran/2012-04/msg00040.html
>
> That one was committed:
> http://gcc.gnu.org/ml/gcc-cvs/2012-04/msg00417.html
>
> Jerry indicated that this would also be OK for a backport; I'll
> do that within a few days unless there are objections.

I prefer if you do not back port it. It is not a regression and it does
not solve a serious deficit. It only seems to be for very special cases.
In addition, the results shown by Dominique and Manfred support the
caution: While there is no speedup, there is now the use of an
uninitialized value.


> A more general question: I habe been a bit inconsistent in notifying
> the mailing list about committed patches; I didn't do this for these
> patches. What would people, generally, prefer?  Should we notify on
> commit,
> or rather not?

I think that it is usually not necessary as one can quickly check the
ChangeLog(s) and usually the time between submittal, approval and
committal is relatively short. But I don't mind if someone mentions at
the mailing list the committal. On the other hand, "committed as
obvious" committals should be send to the mailing list - with the patch.

However, currently, the patch review is a bit shaky. Thus, I thought it
makes sense to create a list of pending patches.

  * * *

Updated list of pending patches:

First, I would like to ping my patch:
- [Patch, Fortran] PR52864 - fix actual/formal checks
   http://gcc.gnu.org/ml/fortran/2012-04/msg00059.html

Other patches with pending review:
- [Patch, Fortran, F03] PR52909: Procedure pointers not private to modules
   http://gcc.gnu.org/ml/fortran/2012-04/msg00033.html
   Caveat: ABI breakage
- [patch, fortran] PR fortran/52537 Optimize string comparisons against empty strings
   http://gcc.gnu.org/ml/fortran/2012-04/msg00068.html
- [Patch, libfortran] Combine get_mem and internal_malloc_size
   http://gcc.gnu.org/ml/fortran/2012-03/msg00127.html


Approved but not yet committed:

My patch:
- [Patch, Fortran] PR52864 - Fix pointer-intent regresssion
   http://gcc.gnu.org/ml/fortran/2012-04/msg00058.html
  <http://gcc.gnu.org/ml/fortran/2012-04/msg00058.html>   (Backporting pending)

Bernhard:
- [PATCH] gfortran testsuite: implicitly cleanup-modules, part 2
   http://gcc.gnu.org/ml/fortran/2012-04/msg00065.html

Janne:
- [Patch, fortran] PR 49010/24518 MOD/MODULO fixes
   http://gcc.gnu.org/ml/fortran/2012-04/msg00012.html
   Okayed but haven't found best wording.


Tobias

[Janne Blomqvist-3]

On Mon, Apr 16, 2012 at 11:03, Tobias Burnus
<tobias.burnus@...> wrote:
> Other patches with pending review:
> - [Patch, libfortran] Combine get_mem and internal_malloc_size
>  http://gcc.gnu.org/ml/fortran/2012-03/msg00127.html

As I said in the original submission, "While the patch is large, it's
also mechanical, hence committed as obvious."

> Approved but not yet committed:

> Janne:
> - [Patch, fortran] PR 49010/24518 MOD/MODULO fixes
>  http://gcc.gnu.org/ml/fortran/2012-04/msg00012.html
>  Okayed but haven't found best wording.

I have an IMHO better wording, namely for MOD(A,P) "the returned value
has the same sign as A and a magnitude less than the magnitude of P."
and for MODULO(A,P) "the returned value has the same sign as P and a
magnitude less than the magnitude of P.". This wording implies what
the sign of the result must be when A is (+-) 0.0 like it does for any
other finite A, which I think is nice.

However, in order to implement this wording, MODULO needs to be
implemented a bit differently than now, namely now we have

res = fmod(a, p);
if (res && ((a < 0) != (p < 0))
  res += p;

but in order to ensure the behavior above for signed zero we need to do

res = fmod(a, p);
if (res)
  {
     if ((a < 0) != (p < 0))
       res += p;
  }
else
  res = copysign (0.0, p);

I have implemented the compile-time part of this, but I haven't yet
had the time to do the runtime code generation (which should be
conditional on -fsigned-zeros). I'll resubmit when done.

--
Janne Blomqvist

[Bernhard Reutner-Fischer]

On Tue, Apr 17, 2012 at 12:47:48AM +0200, Tobias Burnus wrote:

>Approved but not yet committed:

>Bernhard:
>- [PATCH] gfortran testsuite: implicitly cleanup-modules, part 2
>  http://gcc.gnu.org/ml/fortran/2012-04/msg00065.html

Before actually pushing this, I ment to ask if we *want* to make
sure that we do not add superfluous cleanup-module calls in the
future (which would slow down testing needlessly)?

If so we would either have to manually reject occurances of those during
patch-review or install a warning or the like if there is an explicit
cleanup-modules call yielding the same set as the now automatically
determined set.

Since i have not yet looked into the automatic warning myself i would
have hoped that you would not add more explicit cleanup-module calls but
i guess this will not really work out, long-term :)

Thoughts?

[Mike Stump-3]

On Apr 18, 2012, at 9:57 AM, Bernhard Reutner-Fischer wrote:
> Before actually pushing this, I ment to ask if we *want* to make
> sure that we do not add superfluous cleanup-module calls in the
> future (which would slow down testing needlessly)?

I'd run you script again in another 6 months, and once again in a year, that'd probably catch 99% of what a check would catch, and runs faster for 99% of the people.

> Since i have not yet looked into the automatic warning myself i would
> have hoped that you would not add more explicit cleanup-module calls but
> i guess this will not really work out, long-term :)

Sure it will, you remove the documentation for it (or make it clear when it isn't necessary), and beat everyone on the head that does it. Usually, you'd only have to bonk each person twice, then, you're done.  I can't imagine more than 30 bonks would be necessary to train them up.  :-)

[Tobias Burnus]

On 18 April 2012 at 18:57, Bernhard Reutner-Fischer wrote:

> On Tue, Apr 17, 2012 at 12:47:48AM +0200, Tobias Burnus wrote:
>> Approved but not yet committed:
>> Bernhard:
>> - [PATCH] gfortran testsuite: implicitly cleanup-modules, part 2
>>   http://gcc.gnu.org/ml/fortran/2012-04/msg00065.html
> Before actually pushing this, I ment to ask if we *want* to make
> sure that we do not add superfluous cleanup-module calls in the
> future (which would slow down testing needlessly)?
>
> If so we would either have to manually reject occurances of those during
> patch-review or install a warning or the like if there is an explicit
> cleanup-modules call yielding the same set as the now automatically
> determined set.

I would go for the manual method: As cleanup-modules is something which
developers tend to forget, I do not think that many patches will include
them. On then simply tries to reduce those by patch review. - If patch
developers do not see it in other files, the chance is high that they do
not even know (or remember) about that feature in a few months.

And after some time (1/2 year, 1 year?), one can check whether a
spurious clean-up modules has slipped in - or whether some
cleanup-module is missing. I expect that there will be none or very,
very few cases.

Tobias

[Bernhard Reutner-Fischer]

On Fri, May 11, 2012 at 07:34:30PM +0200, Tobias Burnus wrote:

>On 18 April 2012 at 18:57, Bernhard Reutner-Fischer wrote:
>>On Tue, Apr 17, 2012 at 12:47:48AM +0200, Tobias Burnus wrote:
>>>Approved but not yet committed:
>>>Bernhard:
>>>- [PATCH] gfortran testsuite: implicitly cleanup-modules, part 2
>>>  http://gcc.gnu.org/ml/fortran/2012-04/msg00065.html
>>Before actually pushing this, I ment to ask if we *want* to make
>>sure that we do not add superfluous cleanup-module calls in the
>>future (which would slow down testing needlessly)?
>>
>>If so we would either have to manually reject occurances of those during
>>patch-review or install a warning or the like if there is an explicit
>>cleanup-modules call yielding the same set as the now automatically
>>determined set.
>
>I would go for the manual method: As cleanup-modules is something
>which developers tend to forget, I do not think that many patches
>will include them. On then simply tries to reduce those by patch
>review. - If patch developers do not see it in other files, the
>chance is high that they do not even know (or remember) about that
>feature in a few months.
>
>And after some time (1/2 year, 1 year?), one can check whether a
>spurious clean-up modules has slipped in - or whether some
>cleanup-module is missing. I expect that there will be none or very,
>very few cases.

I have committed this as r187521.

thanks,