Bug 29600

Summary: [F03] MINLOC and MAXLOC take an optional KIND argument
Product: gcc Reporter: Francois-Xavier Coudert <fxcoudert>
Component: fortranAssignee: Not yet assigned to anyone <unassigned>
Status: RESOLVED FIXED    
Severity: enhancement CC: dfranke, gcc-bugs, janus, jvdelisle2, tkoenig
Priority: P3 Keywords: rejects-valid
Version: 4.3.0   
Target Milestone: ---   
Host: Target:
Build: Known to work:
Known to fail: Last reconfirmed: 2009-01-13 19:24:06
Bug Depends on: 82660    
Bug Blocks: 20585    
Attachments: Patch which works for maxloc except for some cases
Patch that works mostly

Description Francois-Xavier Coudert 2006-10-26 08:22:04 UTC
The following intrinsics were given an additional (optional) KIND argument by F2003: ACHAR, COUNT, IACHAR, ICHAR, INDEX, LBOUND, LEN, LEN_TRIM, MAXLOC, MINLOC, SCAN, SHAPE, SIZE, UBOUND, VERIFY
Comment 1 Francois-Xavier Coudert 2007-08-08 17:37:36 UTC
The following are easy to fix (I have a patch that just needs writing doc, I'll submit later today): COUNT, IACHAR, ICHAR, INDEX, LBOUND, LEN, LEN_TRIM, SCAN, SIZE, UBOUND, VERIFY.

This will leave: ACHAR (not so hard), MAXLOC, MINLOC and SHAPE. The last three are a bit more tricky if we want to avoid combinatorial explosion of the library routines ;-)
Comment 2 Francois-Xavier Coudert 2007-08-12 19:57:13 UTC
Subject: Bug 29600

Author: fxcoudert
Date: Sun Aug 12 19:57:01 2007
New Revision: 127380

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127380
Log:
	PR fortran/29600

	* intrinsic.c (add_functions): Add KIND arguments to COUNT,
	IACHAR, ICHAR, INDEX, LBOUND, LEN, LEN_TRIM, SCAN, SIZE, UBOUND
	and VERIFY.
	* iresolve.c (gfc_resolve_count): Add kind argument.
	(gfc_resolve_iachar): New function.
	(gfc_resolve_ichar): Add kind argument.
	(gfc_resolve_index_func): Likewise.
	(gfc_resolve_lbound): Likewise.
	(gfc_resolve_len): Likewise.
	(gfc_resolve_len_trim): Likewise.
	(gfc_resolve_scan): Likewise.
	(gfc_resolve_size): New function.
	(gfc_resolve_ubound): Add kind argument.
	(gfc_resolve_verify): Likewise.
	* trans-decl.c (gfc_get_extern_function_decl): Allow specific
	intrinsics to have 4 arguments.
	* check.c (gfc_check_count): Add kind argument.
	(gfc_check_ichar_iachar): Likewise.
	(gfc_check_index): Likewise.
	(gfc_check_lbound): Likewise.
	(gfc_check_len_lentrim): New function.
	(gfc_check_scan): Add kind argument.
	(gfc_check_size): Likewise.
	(gfc_check_ubound): Likewise.
	(gfc_check_verify): Likewise.
	* intrinsic.texi: Update documentation for COUNT, IACHAR, ICHAR,
	INDEX, LBOUND, LEN, LEN_TRIM, SCAN, SIZE, UBOUND and VERIFY.
	* simplify.c (get_kind): Whitespace fix.
	(int_expr_with_kind): New function.
	(gfc_simplify_iachar): Add kind argument.
	(gfc_simplify_iachar): Likewise.
	(gfc_simplify_ichar): Likewise.
	(gfc_simplify_index): Likewise.
	(simplify_bound_dim): Likewise.
	(simplify_bound): Likewise.
	(gfc_simplify_lbound): Likewise.
	(gfc_simplify_len): Likewise.
	(gfc_simplify_len_trim): Likewise.
	(gfc_simplify_scan): Likewise.
	(gfc_simplify_shape): Pass NULL as kind argument to gfc_simplify_size.
	(gfc_simplify_size): Add kind argument.
	(gfc_simplify_ubound): Likewise.
	(gfc_simplify_verify): Likewise.
	* intrinsic.h: Update prototypes and add new ones.
	* trans-intrinsic.c (gfc_conv_intrinsic_index): Rename into
	gfc_conv_intrinsic_index_scan_verify.
	(gfc_conv_intrinsic_scan, gfc_conv_intrinsic_verify): Remove.
	(gfc_conv_intrinsic_function): Call
	gfc_conv_intrinsic_index_scan_verify to translate the INDEX,
	SCAN and VERIFY intrinsics.

	* gfortran.dg/intrinsics_kind_argument_1.f90: New test.
	* gfortran.dg/pure_dummy_length_1.f90: Adapt to new error wording.

Added:
    trunk/gcc/testsuite/gfortran.dg/intrinsics_kind_argument_1.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/check.c
    trunk/gcc/fortran/intrinsic.c
    trunk/gcc/fortran/intrinsic.h
    trunk/gcc/fortran/intrinsic.texi
    trunk/gcc/fortran/iresolve.c
    trunk/gcc/fortran/simplify.c
    trunk/gcc/fortran/trans-decl.c
    trunk/gcc/fortran/trans-intrinsic.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/pure_dummy_length_1.f90

Comment 3 Francois-Xavier Coudert 2007-08-12 21:21:25 UTC
Subject: Bug 29600

Author: fxcoudert
Date: Sun Aug 12 21:21:08 2007
New Revision: 127385

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=127385
Log:
	PR fortran/29600

	* intrinsic.c (add_functions): Add optional KIND argument to ACHAR.
	* iresolve.c (gfc_resolve_achar): Handle the KIND argument.
	* check.c (gfc_check_achar): Check for the optional KIND argument.
	* simplify.c (gfc_simplify_achar): Use KIND argument.
	* intrinsic.h (gfc_check_achar, gfc_simplify_achar,
	gfc_resolve_achar): Adjust prototypes.

	* gfortran.dg/intrinsics_kind_argument_1.f90: Add test for ACHAR
	intrinsic.

Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/check.c
    trunk/gcc/fortran/intrinsic.c
    trunk/gcc/fortran/intrinsic.h
    trunk/gcc/fortran/iresolve.c
    trunk/gcc/fortran/simplify.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gfortran.dg/intrinsics_kind_argument_1.f90

Comment 4 Francois-Xavier Coudert 2007-08-12 21:26:42 UTC
Updated summary to reflect current state: only MINLOC, MAXLOC and SHAPE needs to be done. I'm not working on it any more.
Comment 5 Thomas Koenig 2008-01-20 09:50:52 UTC
I'll take a shot at this.
Comment 6 Janne Blomqvist 2008-05-22 18:13:36 UTC
The "proper" solution, which as a side effect would take care of the combinatorial explosion issue as well, would be to inline these intrinsics. See also PR 31067.
Comment 7 Thomas Koenig 2008-06-04 19:06:03 UTC
No time for now (Real Time is catching up with me big time).

Unassigning (for now).
Comment 8 Daniel Franke 2009-01-04 00:10:17 UTC
See also: PR36462.
Comment 9 Daniel Franke 2009-03-31 19:44:35 UTC
Patch:
  http://gcc.gnu.org/ml/fortran/2009-03/msg00142.html

Rejected due to concerns about additional bloat of the library (see also #6).
The frontend-changes are (probably) ok.

Unassigning myself.
Comment 10 Francois-Xavier Coudert 2015-08-26 12:38:14 UTC
Author: fxcoudert
Date: Wed Aug 26 12:37:42 2015
New Revision: 227210

URL: https://gcc.gnu.org/viewcvs?rev=227210&root=gcc&view=rev
Log:
	PR fortran/29600

	* Makefile.am: Add generated/shape_i{1,2}.c
	* Makefile.in: Regenerate.
	* generated/shape_i1.c: New generated file.
	* generated/shape_i2.c: New generated file.
	* generated/shape_i4.c: Regenerate.
	* generated/shape_i8.c: Regenerate.
	* generated/shape_i16.c: Regenerate.
	* gfortran.map (GFORTRAN_1.7): Add _gfortran_shape_{1,2}.
	* m4/shape.m4: Fix parameter type.

	* gfortran.dg/shape_8.f90: New test.

Added:
    trunk/gcc/testsuite/gfortran.dg/shape_8.f90
    trunk/libgfortran/generated/shape_i1.c
    trunk/libgfortran/generated/shape_i2.c
Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/Makefile.am
    trunk/libgfortran/Makefile.in
    trunk/libgfortran/generated/shape_i16.c
    trunk/libgfortran/generated/shape_i4.c
    trunk/libgfortran/generated/shape_i8.c
    trunk/libgfortran/gfortran.map
    trunk/libgfortran/m4/shape.m4
Comment 11 Francois-Xavier Coudert 2015-08-26 12:40:11 UTC
SHAPE is taken care of. Still remain MINLOC and MAXLOC.
Comment 12 Thomas Koenig 2017-10-21 15:24:49 UTC
The best thing to do would be to only have the version with
index_type, and then convert the result if necessary.
Comment 13 Thomas Koenig 2017-10-21 16:14:48 UTC
Created attachment 42429 [details]
Patch which works for maxloc except for some cases

This patch works, sort of, but it fails with a segfault for

program main
  real, dimension(3) :: a
  call random_number(a)

  print *, maxloc(a)
end program main

Let's try some more things here...
Comment 14 Thomas Koenig 2017-10-22 13:28:47 UTC
Created attachment 42434 [details]
Patch that works mostly

This patch mostly works except that

- it does not yet remove unneeded library functions
- The conversion makes some bounds checking cases fail
  (see PR82660).
Comment 15 Thomas Koenig 2017-10-22 13:40:43 UTC
Out of ideas at the moment, unassigning myself.
Comment 16 Jerry DeLisle 2017-10-27 02:37:18 UTC
(In reply to Thomas Koenig from comment #15)
> Out of ideas at the moment, unassigning myself.

Without looking at your attempts, it seems one could parse in the scanner for whether there is this optional argumanet. If not, set it to NULL and in that case do what we do now, otherwise do a kind conversion.  Is this what you tried?

Runtime would just check for the parameter and do it tat way. 

Or are you trying to inline this or do it all in the frontend?
Comment 17 Thomas Koenig 2017-10-27 13:59:04 UTC
What I am trying to do is to have only one version of each
library routine (the one using gfc_index_kind), and then
convert to the KIND parameter that the user specified (or to
default integer if the user didn't specify anything). This would
also allow to reduce library bloat.

This works fine with the attached patch. I did not yet touch the
library, but removing the then-unneeded versions of the library
functions would be fairly trivial. We're breaking binary
compatibility anyway, so we might as well do that.

The only problem is that it causes regressions in bounds
checking - the __convert_i8_i4 function call (in case where
gfc_index_type has 8 bytes and normal integers have 4 bytes).
This is a more or less unrelated problem, and is PR82660.

Now, we could argue that the bounds violations for minloc and
maxloc are not very important, since the dimension of the
return array is the rank of the argument - in the normal
case, compile-time checking will catch the problem anyway.
We would then accept a regression in return for implementing
an (IMHO) important part of F2003 - without these KIND argumetns,
MINLOC and MAXLOC are useless for long linear arrays with > 2^31
elements.

Of course, we should also focus on PR 82660.

What do you think?
Comment 18 Thomas Koenig 2017-11-04 13:21:04 UTC
Author: tkoenig
Date: Sat Nov  4 13:20:32 2017
New Revision: 254405

URL: https://gcc.gnu.org/viewcvs?rev=254405&root=gcc&view=rev
Log:
2017-11-04  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/29600
	* gfortran.h (gfc_check_f): Replace fm3l with fm4l.
	* intrinsic.h (gfc_resolve_maxloc): Add gfc_expr * to argument
	list in protoytpe.
	(gfc_resolve_minloc): Likewise.
	* check.c (gfc_check_minloc_maxloc): Handle kind argument.
	* intrinsic.c (add_sym_3_ml): Rename to
	(add_sym_4_ml): and handle kind argument.
	(add_function): Replace add_sym_3ml with add_sym_4ml and add
	extra arguments for maxloc and minloc.
	(check_specific): Change use of check.f3ml with check.f4ml.
	* iresolve.c (gfc_resolve_maxloc): Handle kind argument. If
	the kind is smaller than the smallest library version available,
	use gfc_default_integer_kind and convert afterwards.
	(gfc_resolve_minloc): Likewise.

2017-11-04  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/29600
	* gfortran.dg/minmaxloc_8.f90: New test.


Added:
    trunk/gcc/testsuite/gfortran.dg/minmaxloc_8.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/check.c
    trunk/gcc/fortran/gfortran.h
    trunk/gcc/fortran/intrinsic.c
    trunk/gcc/fortran/intrinsic.h
    trunk/gcc/fortran/iresolve.c
    trunk/gcc/testsuite/ChangeLog
Comment 19 Thomas Koenig 2017-11-04 13:23:10 UTC
Fixed on trunk, closing.
Comment 20 Thomas Koenig 2017-11-05 21:34:03 UTC
Really closing.