public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
@ 2016-11-18 14:05 Dominique d'Humières
  2016-11-18 21:06 ` Janus Weil
  0 siblings, 1 reply; 11+ messages in thread
From: Dominique d'Humières @ 2016-11-18 14:05 UTC (permalink / raw)
  To: Janus Weil; +Cc: fortran, gcc-patches

Hi Janus,

> the attached patch fixes an ice-on-valid problem, simply by removing an assert. ...

I have several instances in my test suite showing that the proposed patch removes the ICE but generates wrong code:

pr42359, second test, => ICE on another place
pr54613, sixth and eighth tests,

Thanks for working on the issue,

Dominique

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
  2016-11-18 14:05 [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979 Dominique d'Humières
@ 2016-11-18 21:06 ` Janus Weil
  2016-11-19  9:12   ` Janus Weil
  0 siblings, 1 reply; 11+ messages in thread
From: Janus Weil @ 2016-11-18 21:06 UTC (permalink / raw)
  To: Dominique d'Humières; +Cc: gfortran, gcc-patches

Hi Dominique,

>> the attached patch fixes an ice-on-valid problem, simply by removing an assert. ...
>
> I have several instances in my test suite showing that the proposed patch removes the ICE but generates wrong code:
>
> pr42359, second test, => ICE on another place
> pr54613, sixth and eighth tests,

thanks for the comments, those cases are closely related.

I previously assumed that the test case for this PR would be legal,
but by now I think that's wrong. The test case should be rejected, and
we already have checking mechanisms for this (see
resolve_fl_variable), but apparently they are not working.

My current suspicion is that 'gfc_is_constant_expr' has a bug, because
it claims the call to the function 'get_i' to be a constant
expression. This is not true, because get_i() can not be reduced to a
compile-time constant.

In any case the patch I proposed is wrong and the assert should stay.

Cheers,
Janus

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
  2016-11-18 21:06 ` Janus Weil
@ 2016-11-19  9:12   ` Janus Weil
  2016-11-26  9:45     ` Janus Weil
  0 siblings, 1 reply; 11+ messages in thread
From: Janus Weil @ 2016-11-19  9:12 UTC (permalink / raw)
  To: Dominique d'Humières; +Cc: gfortran, gcc-patches, Jerry DeLisle

[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]

Hi all,

> I previously assumed that the test case for this PR would be legal,
> but by now I think that's wrong. The test case should be rejected, and
> we already have checking mechanisms for this (see
> resolve_fl_variable), but apparently they are not working.
>
> My current suspicion is that 'gfc_is_constant_expr' has a bug, because
> it claims the call to the function 'get_i' to be a constant
> expression. This is not true, because get_i() can not be reduced to a
> compile-time constant.

some more reading in the standard confirms this suspicion: In
gfc_is_constant_expr there is a piece of code which claims that
specification functions are constant. That is certainly not true, and
so what I'm doing in the attached fix is to remove that code and add
some references to the standard to make things clearer.

The code that I'm removing has last been touched in this commit by
Jerry six years ago:

https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=166520

However, this did not introduce the bug in the first place (not sure
when that happened).

In any case the new patch in the attachment regtests cleanly and
correctly rejects the original test case as well as one of the cases
mentioned by Dominique. Ok for trunk?

Cheers,
Janus



2016-11-19  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/78392
    * expr.c (gfc_is_constant_expr): Specification functions are not
    compile-time constants. Update documentation (add reference to F08
    standard), add a FIXME.
    (external_spec_function): Add reference to F08 standard.
    * resolve.c (resolve_fl_variable): Ditto.

2016-11-19  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/78392
    * gfortran.dg/constant_shape.f90: New test case.

[-- Attachment #2: pr78392_v3.diff --]
[-- Type: text/plain, Size: 2537 bytes --]

Index: gcc/fortran/expr.c
===================================================================
--- gcc/fortran/expr.c	(Revision 242620)
+++ gcc/fortran/expr.c	(Arbeitskopie)
@@ -883,8 +883,9 @@ done:
 }
 
 
-/* Function to determine if an expression is constant or not.  This
-   function expects that the expression has already been simplified.  */
+/* Determine if an expression is constant in the sense of F08:7.1.12.
+ * This function expects that the expression has already been simplified.
+ * FIXME: Return a bool, not an int.  */
 
 int
 gfc_is_constant_expr (gfc_expr *e)
@@ -891,7 +892,6 @@ gfc_is_constant_expr (gfc_expr *e)
 {
   gfc_constructor *c;
   gfc_actual_arglist *arg;
-  gfc_symbol *sym;
 
   if (e == NULL)
     return 1;
@@ -920,25 +920,6 @@ gfc_is_constant_expr (gfc_expr *e)
 	      return 0;
 	}
 
-      /* Specification functions are constant.  */
-      /* F95, 7.1.6.2; F2003, 7.1.7  */
-      sym = NULL;
-      if (e->symtree)
-	sym = e->symtree->n.sym;
-      if (e->value.function.esym)
-	sym = e->value.function.esym;
-
-      if (sym
-	  && sym->attr.function
-	  && sym->attr.pure
-	  && !sym->attr.intrinsic
-	  && !sym->attr.recursive
-	  && sym->attr.proc != PROC_INTERNAL
-	  && sym->attr.proc != PROC_ST_FUNCTION
-	  && sym->attr.proc != PROC_UNKNOWN
-	  && gfc_sym_get_dummy_args (sym) == NULL)
-	return 1;
-
       if (e->value.function.isym
 	  && (e->value.function.isym->elemental
 	      || e->value.function.isym->pure
@@ -2741,7 +2722,8 @@ restricted_args (gfc_actual_arglist *a)
 /************* Restricted/specification expressions *************/
 
 
-/* Make sure a non-intrinsic function is a specification function.  */
+/* Make sure a non-intrinsic function is a specification function,
+ * see F08:7.1.11.5.  */
 
 static bool
 external_spec_function (gfc_expr *e)
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c	(Revision 242620)
+++ gcc/fortran/resolve.c	(Arbeitskopie)
@@ -11831,8 +11831,8 @@ resolve_fl_variable (gfc_symbol *sym, int mp_flag)
       && !sym->attr.pointer
       && is_non_constant_shape_array (sym))
     {
-      /* The shape of a main program or module array needs to be
-	 constant.  */
+      /* F08:C541. The shape of an array defined in a main program or module
+       * needs to be constant.  */
       gfc_error ("The module or main program array %qs at %L must "
 		 "have constant shape", sym->name, &sym->declared_at);
       specification_expr = saved_specification_expr;

[-- Attachment #3: constant_shape.f90 --]
[-- Type: text/x-fortran, Size: 420 bytes --]

! { dg-do compile }
!
! PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
!
! Contributed by Janus Weil <janus@gcc.gnu.org>

module mytypes
   implicit none
 contains
   pure integer function get_i ()
     get_i = 13
   end function
end module

program test
  use mytypes
  implicit none
  integer, dimension(get_i()) :: x  ! { dg-error "must have constant shape" }
  print *, size (x)
end

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
  2016-11-19  9:12   ` Janus Weil
@ 2016-11-26  9:45     ` Janus Weil
  2016-11-26 16:37       ` Dominique d'Humières
  2016-12-03  7:05       ` Janus Weil
  0 siblings, 2 replies; 11+ messages in thread
From: Janus Weil @ 2016-11-26  9:45 UTC (permalink / raw)
  To: Dominique d'Humières; +Cc: gfortran, gcc-patches, Jerry DeLisle

ping!


2016-11-19 10:12 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
> Hi all,
>
>> I previously assumed that the test case for this PR would be legal,
>> but by now I think that's wrong. The test case should be rejected, and
>> we already have checking mechanisms for this (see
>> resolve_fl_variable), but apparently they are not working.
>>
>> My current suspicion is that 'gfc_is_constant_expr' has a bug, because
>> it claims the call to the function 'get_i' to be a constant
>> expression. This is not true, because get_i() can not be reduced to a
>> compile-time constant.
>
> some more reading in the standard confirms this suspicion: In
> gfc_is_constant_expr there is a piece of code which claims that
> specification functions are constant. That is certainly not true, and
> so what I'm doing in the attached fix is to remove that code and add
> some references to the standard to make things clearer.
>
> The code that I'm removing has last been touched in this commit by
> Jerry six years ago:
>
> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=166520
>
> However, this did not introduce the bug in the first place (not sure
> when that happened).
>
> In any case the new patch in the attachment regtests cleanly and
> correctly rejects the original test case as well as one of the cases
> mentioned by Dominique. Ok for trunk?
>
> Cheers,
> Janus
>
>
>
> 2016-11-19  Janus Weil  <janus@gcc.gnu.org>
>
>     PR fortran/78392
>     * expr.c (gfc_is_constant_expr): Specification functions are not
>     compile-time constants. Update documentation (add reference to F08
>     standard), add a FIXME.
>     (external_spec_function): Add reference to F08 standard.
>     * resolve.c (resolve_fl_variable): Ditto.
>
> 2016-11-19  Janus Weil  <janus@gcc.gnu.org>
>
>     PR fortran/78392
>     * gfortran.dg/constant_shape.f90: New test case.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
  2016-11-26  9:45     ` Janus Weil
@ 2016-11-26 16:37       ` Dominique d'Humières
  2016-11-26 18:10         ` Janus Weil
  2016-12-03  7:05       ` Janus Weil
  1 sibling, 1 reply; 11+ messages in thread
From: Dominique d'Humières @ 2016-11-26 16:37 UTC (permalink / raw)
  To: Janus Weil; +Cc: gfortran, gcc-patches, Jerry DeLisle


> Le 26 nov. 2016 à 10:45, Janus Weil <janus@gcc.gnu.org> a écrit :
> 
> ping!
> 
The patch is working has expected. Note the removed block has been introduced by Daniel Franke at r126826.

Dominique.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
  2016-11-26 16:37       ` Dominique d'Humières
@ 2016-11-26 18:10         ` Janus Weil
  0 siblings, 0 replies; 11+ messages in thread
From: Janus Weil @ 2016-11-26 18:10 UTC (permalink / raw)
  To: Dominique d'Humières; +Cc: gfortran, gcc-patches, Jerry DeLisle

2016-11-26 17:37 GMT+01:00 Dominique d'Humières <dominiq@lps.ens.fr>:
>
>> Le 26 nov. 2016 à 10:45, Janus Weil <janus@gcc.gnu.org> a écrit :
>>
>> ping!
>>
> The patch is working has expected. Note the removed block has been introduced by Daniel Franke at r126826.

Right, thanks for the reference. I think that commit is plain wrong,
at least the part that says "Specification functions are constant".

One can easily construct a specification function that is not a
compile-time constant. For example, just take the module function
"get_i" in the test case and have it depend on a variable declared in
the module header.

module mytypes
   implicit none
   integer, save :: i = 13
 contains
   pure integer function get_i ()
     get_i = i
   end function
  subroutine set_i (j)
    integer, intent(in) :: j
    i = j
  end subroutine
end module

Cheers,
Janus

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
  2016-11-26  9:45     ` Janus Weil
  2016-11-26 16:37       ` Dominique d'Humières
@ 2016-12-03  7:05       ` Janus Weil
  2016-12-12 15:52         ` Janus Weil
  1 sibling, 1 reply; 11+ messages in thread
From: Janus Weil @ 2016-12-03  7:05 UTC (permalink / raw)
  To: Dominique d'Humières; +Cc: gfortran, gcc-patches, Jerry DeLisle

double-ping!


2016-11-26 10:45 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
> ping!
>
>
> 2016-11-19 10:12 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
>> Hi all,
>>
>>> I previously assumed that the test case for this PR would be legal,
>>> but by now I think that's wrong. The test case should be rejected, and
>>> we already have checking mechanisms for this (see
>>> resolve_fl_variable), but apparently they are not working.
>>>
>>> My current suspicion is that 'gfc_is_constant_expr' has a bug, because
>>> it claims the call to the function 'get_i' to be a constant
>>> expression. This is not true, because get_i() can not be reduced to a
>>> compile-time constant.
>>
>> some more reading in the standard confirms this suspicion: In
>> gfc_is_constant_expr there is a piece of code which claims that
>> specification functions are constant. That is certainly not true, and
>> so what I'm doing in the attached fix is to remove that code and add
>> some references to the standard to make things clearer.
>>
>> The code that I'm removing has last been touched in this commit by
>> Jerry six years ago:
>>
>> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=166520
>>
>> However, this did not introduce the bug in the first place (not sure
>> when that happened).
>>
>> In any case the new patch in the attachment regtests cleanly and
>> correctly rejects the original test case as well as one of the cases
>> mentioned by Dominique. Ok for trunk?
>>
>> Cheers,
>> Janus
>>
>>
>>
>> 2016-11-19  Janus Weil  <janus@gcc.gnu.org>
>>
>>     PR fortran/78392
>>     * expr.c (gfc_is_constant_expr): Specification functions are not
>>     compile-time constants. Update documentation (add reference to F08
>>     standard), add a FIXME.
>>     (external_spec_function): Add reference to F08 standard.
>>     * resolve.c (resolve_fl_variable): Ditto.
>>
>> 2016-11-19  Janus Weil  <janus@gcc.gnu.org>
>>
>>     PR fortran/78392
>>     * gfortran.dg/constant_shape.f90: New test case.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
  2016-12-03  7:05       ` Janus Weil
@ 2016-12-12 15:52         ` Janus Weil
  2016-12-12 17:37           ` Paul Richard Thomas
  0 siblings, 1 reply; 11+ messages in thread
From: Janus Weil @ 2016-12-12 15:52 UTC (permalink / raw)
  To: Dominique d'Humières; +Cc: gfortran, gcc-patches, Jerry DeLisle

Hi all,

I hate to ping this patch once more, but somehow we need to come to a
conclusion here.

The issue boils down to the fact that there is a piece of code in the
gfortran code which claims that specification functions are
'constant', but I doubt that this is true. To my understanding the
concept of specification expressions and specification functions (see
section 7.1.6 in the F03 standard) was introduced essentially to refer
to side-effect-free expressions that can be used e.g. in a
type-specification context (array bounds, char-length parameters etc).

However I think 'specification functions' do not necessarily need to
be 'constants', in the sense that subsequent invocations give always
the same (constant) result and their value can be determined at
compile time.

My patch is at: https://gcc.gnu.org/ml/fortran/2016-11/msg00188.html
Further discussion at: https://gcc.gnu.org/ml/fortran/2016-11/msg00243.html

Any comments, please!?!

Cheers,
Janus



2016-12-03 8:05 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
> double-ping!
>
>
> 2016-11-26 10:45 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
>> ping!
>>
>>
>> 2016-11-19 10:12 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
>>> Hi all,
>>>
>>>> I previously assumed that the test case for this PR would be legal,
>>>> but by now I think that's wrong. The test case should be rejected, and
>>>> we already have checking mechanisms for this (see
>>>> resolve_fl_variable), but apparently they are not working.
>>>>
>>>> My current suspicion is that 'gfc_is_constant_expr' has a bug, because
>>>> it claims the call to the function 'get_i' to be a constant
>>>> expression. This is not true, because get_i() can not be reduced to a
>>>> compile-time constant.
>>>
>>> some more reading in the standard confirms this suspicion: In
>>> gfc_is_constant_expr there is a piece of code which claims that
>>> specification functions are constant. That is certainly not true, and
>>> so what I'm doing in the attached fix is to remove that code and add
>>> some references to the standard to make things clearer.
>>>
>>> The code that I'm removing has last been touched in this commit by
>>> Jerry six years ago:
>>>
>>> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=166520
>>>
>>> However, this did not introduce the bug in the first place (not sure
>>> when that happened).
>>>
>>> In any case the new patch in the attachment regtests cleanly and
>>> correctly rejects the original test case as well as one of the cases
>>> mentioned by Dominique. Ok for trunk?
>>>
>>> Cheers,
>>> Janus
>>>
>>>
>>>
>>> 2016-11-19  Janus Weil  <janus@gcc.gnu.org>
>>>
>>>     PR fortran/78392
>>>     * expr.c (gfc_is_constant_expr): Specification functions are not
>>>     compile-time constants. Update documentation (add reference to F08
>>>     standard), add a FIXME.
>>>     (external_spec_function): Add reference to F08 standard.
>>>     * resolve.c (resolve_fl_variable): Ditto.
>>>
>>> 2016-11-19  Janus Weil  <janus@gcc.gnu.org>
>>>
>>>     PR fortran/78392
>>>     * gfortran.dg/constant_shape.f90: New test case.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
  2016-12-12 15:52         ` Janus Weil
@ 2016-12-12 17:37           ` Paul Richard Thomas
  2016-12-12 18:56             ` Janus Weil
  0 siblings, 1 reply; 11+ messages in thread
From: Paul Richard Thomas @ 2016-12-12 17:37 UTC (permalink / raw)
  To: Janus Weil
  Cc: Dominique d'Humières, gfortran, gcc-patches, Jerry DeLisle

Hi Janus,

The patch is good - OK for trunk.

Thanks

Paul

On 12 December 2016 at 16:52, Janus Weil <janus@gcc.gnu.org> wrote:
> Hi all,
>
> I hate to ping this patch once more, but somehow we need to come to a
> conclusion here.
>
> The issue boils down to the fact that there is a piece of code in the
> gfortran code which claims that specification functions are
> 'constant', but I doubt that this is true. To my understanding the
> concept of specification expressions and specification functions (see
> section 7.1.6 in the F03 standard) was introduced essentially to refer
> to side-effect-free expressions that can be used e.g. in a
> type-specification context (array bounds, char-length parameters etc).
>
> However I think 'specification functions' do not necessarily need to
> be 'constants', in the sense that subsequent invocations give always
> the same (constant) result and their value can be determined at
> compile time.
>
> My patch is at: https://gcc.gnu.org/ml/fortran/2016-11/msg00188.html
> Further discussion at: https://gcc.gnu.org/ml/fortran/2016-11/msg00243.html
>
> Any comments, please!?!
>
> Cheers,
> Janus
>
>
>
> 2016-12-03 8:05 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
>> double-ping!
>>
>>
>> 2016-11-26 10:45 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
>>> ping!
>>>
>>>
>>> 2016-11-19 10:12 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
>>>> Hi all,
>>>>
>>>>> I previously assumed that the test case for this PR would be legal,
>>>>> but by now I think that's wrong. The test case should be rejected, and
>>>>> we already have checking mechanisms for this (see
>>>>> resolve_fl_variable), but apparently they are not working.
>>>>>
>>>>> My current suspicion is that 'gfc_is_constant_expr' has a bug, because
>>>>> it claims the call to the function 'get_i' to be a constant
>>>>> expression. This is not true, because get_i() can not be reduced to a
>>>>> compile-time constant.
>>>>
>>>> some more reading in the standard confirms this suspicion: In
>>>> gfc_is_constant_expr there is a piece of code which claims that
>>>> specification functions are constant. That is certainly not true, and
>>>> so what I'm doing in the attached fix is to remove that code and add
>>>> some references to the standard to make things clearer.
>>>>
>>>> The code that I'm removing has last been touched in this commit by
>>>> Jerry six years ago:
>>>>
>>>> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=166520
>>>>
>>>> However, this did not introduce the bug in the first place (not sure
>>>> when that happened).
>>>>
>>>> In any case the new patch in the attachment regtests cleanly and
>>>> correctly rejects the original test case as well as one of the cases
>>>> mentioned by Dominique. Ok for trunk?
>>>>
>>>> Cheers,
>>>> Janus
>>>>
>>>>
>>>>
>>>> 2016-11-19  Janus Weil  <janus@gcc.gnu.org>
>>>>
>>>>     PR fortran/78392
>>>>     * expr.c (gfc_is_constant_expr): Specification functions are not
>>>>     compile-time constants. Update documentation (add reference to F08
>>>>     standard), add a FIXME.
>>>>     (external_spec_function): Add reference to F08 standard.
>>>>     * resolve.c (resolve_fl_variable): Ditto.
>>>>
>>>> 2016-11-19  Janus Weil  <janus@gcc.gnu.org>
>>>>
>>>>     PR fortran/78392
>>>>     * gfortran.dg/constant_shape.f90: New test case.



-- 
If you're walking down the right path and you're willing to keep
walking, eventually you'll make progress.

Barack Obama

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
  2016-12-12 17:37           ` Paul Richard Thomas
@ 2016-12-12 18:56             ` Janus Weil
  0 siblings, 0 replies; 11+ messages in thread
From: Janus Weil @ 2016-12-12 18:56 UTC (permalink / raw)
  To: Paul Richard Thomas
  Cc: Dominique d'Humières, gfortran, gcc-patches, Jerry DeLisle

2016-12-12 18:37 GMT+01:00 Paul Richard Thomas <paul.richard.thomas@gmail.com>:
> Hi Janus,
>
> The patch is good - OK for trunk.

Thanks, Paul. Committed as r243580.

Cheers,
Janus



> On 12 December 2016 at 16:52, Janus Weil <janus@gcc.gnu.org> wrote:
>> Hi all,
>>
>> I hate to ping this patch once more, but somehow we need to come to a
>> conclusion here.
>>
>> The issue boils down to the fact that there is a piece of code in the
>> gfortran code which claims that specification functions are
>> 'constant', but I doubt that this is true. To my understanding the
>> concept of specification expressions and specification functions (see
>> section 7.1.6 in the F03 standard) was introduced essentially to refer
>> to side-effect-free expressions that can be used e.g. in a
>> type-specification context (array bounds, char-length parameters etc).
>>
>> However I think 'specification functions' do not necessarily need to
>> be 'constants', in the sense that subsequent invocations give always
>> the same (constant) result and their value can be determined at
>> compile time.
>>
>> My patch is at: https://gcc.gnu.org/ml/fortran/2016-11/msg00188.html
>> Further discussion at: https://gcc.gnu.org/ml/fortran/2016-11/msg00243.html
>>
>> Any comments, please!?!
>>
>> Cheers,
>> Janus
>>
>>
>>
>> 2016-12-03 8:05 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
>>> double-ping!
>>>
>>>
>>> 2016-11-26 10:45 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
>>>> ping!
>>>>
>>>>
>>>> 2016-11-19 10:12 GMT+01:00 Janus Weil <janus@gcc.gnu.org>:
>>>>> Hi all,
>>>>>
>>>>>> I previously assumed that the test case for this PR would be legal,
>>>>>> but by now I think that's wrong. The test case should be rejected, and
>>>>>> we already have checking mechanisms for this (see
>>>>>> resolve_fl_variable), but apparently they are not working.
>>>>>>
>>>>>> My current suspicion is that 'gfc_is_constant_expr' has a bug, because
>>>>>> it claims the call to the function 'get_i' to be a constant
>>>>>> expression. This is not true, because get_i() can not be reduced to a
>>>>>> compile-time constant.
>>>>>
>>>>> some more reading in the standard confirms this suspicion: In
>>>>> gfc_is_constant_expr there is a piece of code which claims that
>>>>> specification functions are constant. That is certainly not true, and
>>>>> so what I'm doing in the attached fix is to remove that code and add
>>>>> some references to the standard to make things clearer.
>>>>>
>>>>> The code that I'm removing has last been touched in this commit by
>>>>> Jerry six years ago:
>>>>>
>>>>> https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=166520
>>>>>
>>>>> However, this did not introduce the bug in the first place (not sure
>>>>> when that happened).
>>>>>
>>>>> In any case the new patch in the attachment regtests cleanly and
>>>>> correctly rejects the original test case as well as one of the cases
>>>>> mentioned by Dominique. Ok for trunk?
>>>>>
>>>>> Cheers,
>>>>> Janus
>>>>>
>>>>>
>>>>>
>>>>> 2016-11-19  Janus Weil  <janus@gcc.gnu.org>
>>>>>
>>>>>     PR fortran/78392
>>>>>     * expr.c (gfc_is_constant_expr): Specification functions are not
>>>>>     compile-time constants. Update documentation (add reference to F08
>>>>>     standard), add a FIXME.
>>>>>     (external_spec_function): Add reference to F08 standard.
>>>>>     * resolve.c (resolve_fl_variable): Ditto.
>>>>>
>>>>> 2016-11-19  Janus Weil  <janus@gcc.gnu.org>
>>>>>
>>>>>     PR fortran/78392
>>>>>     * gfortran.dg/constant_shape.f90: New test case.
>
>
>
> --
> If you're walking down the right path and you're willing to keep
> walking, eventually you'll make progress.
>
> Barack Obama

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
@ 2016-11-18 13:03 Janus Weil
  0 siblings, 0 replies; 11+ messages in thread
From: Janus Weil @ 2016-11-18 13:03 UTC (permalink / raw)
  To: gfortran, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 472 bytes --]

Hi all,

the attached patch fixes an ice-on-valid problem, simply by removing
an assert. The generated code works as expected and the patch regtests
cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus



2016-11-18  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/78392
    * trans-array.c (gfc_trans_auto_array_allocation): Remove an assert.

2016-11-18  Janus Weil  <janus@gcc.gnu.org>

    PR fortran/78392
    * gfortran.dg/saved_automatic_2.f90: New test case.

[-- Attachment #2: pr78392.diff --]
[-- Type: text/plain, Size: 440 bytes --]

Index: gcc/fortran/trans-array.c
===================================================================
--- gcc/fortran/trans-array.c	(Revision 242586)
+++ gcc/fortran/trans-array.c	(Arbeitskopie)
@@ -5976,7 +5976,6 @@ gfc_trans_auto_array_allocation (tree decl, gfc_sy
   type = TREE_TYPE (type);
 
   gcc_assert (!sym->attr.use_assoc);
-  gcc_assert (!TREE_STATIC (decl));
   gcc_assert (!sym->module);
 
   if (sym->ts.type == BT_CHARACTER

[-- Attachment #3: saved_automatic_2.f90 --]
[-- Type: text/x-fortran, Size: 393 bytes --]

! { dg-do run }
!
! PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979
!
! Contributed by Janus Weil <janus@gcc.gnu.org>

module mytypes
   implicit none
 contains
   pure integer function get_i ()
     get_i = 13
   end function
end module

program test
  use mytypes
  implicit none
  integer, dimension(get_i()), save :: x
  if (size(x) /= 13) call abort()
end

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-12-12 18:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-18 14:05 [Patch, Fortran] PR 78392: ICE in gfc_trans_auto_array_allocation, at fortran/trans-array.c:5979 Dominique d'Humières
2016-11-18 21:06 ` Janus Weil
2016-11-19  9:12   ` Janus Weil
2016-11-26  9:45     ` Janus Weil
2016-11-26 16:37       ` Dominique d'Humières
2016-11-26 18:10         ` Janus Weil
2016-12-03  7:05       ` Janus Weil
2016-12-12 15:52         ` Janus Weil
2016-12-12 17:37           ` Paul Richard Thomas
2016-12-12 18:56             ` Janus Weil
  -- strict thread matches above, loose matches on Subject: below --
2016-11-18 13:03 Janus Weil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).