Re: [PATCH] Automatics in equivalence statements

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Automatics in equivalence statements

Bernhard Reutner-Fischer
On Fri, 21 Jun 2019 07:10:11 -0700
Steve Kargl <[hidden email]> wrote:

> On Fri, Jun 21, 2019 at 02:31:51PM +0100, Mark Eggleston wrote:
> > Currently variables with the AUTOMATIC attribute can not appear in an
> > EQUIVALENCE statement. However its counterpart, STATIC, can be used in
> > an EQUIVALENCE statement.
> >
> > Where there is a clear conflict in the attributes of variables in an
> > EQUIVALENCE statement an error message will be issued as is currently
> > the case.
> >
> > If there is no conflict e.g. a variable with a AUTOMATIC attribute and a
> > variable(s) without attributes all variables in the EQUIVALENCE will
> > become AUTOMATIC.
> >
> > Note: most of this patch was written by Jeff Law <[hidden email]>
> >
> > Please review.
> >
> > ChangeLogs:
> >
> > gcc/fortran
> >
> >      Jeff Law  <[hidden email]>
> >      Mark Eggleston  <[hidden email]>
> >
> >      * gfortran.h: Add check_conflict declaration.  
>
> This is wrong.  By convention a routine that is not static
> has the gfc_ prefix.
>
Furthermore doesn't this export indicate that you're committing a
layering violation somehow?

>      * symbol.c (check_conflict): Remove automatic in equivalence conflict
>      check.
>      * symbol.c (save_symbol): Add check for in equivalence to stop the
>      the save attribute being added.
>      * trans-common.c (build_equiv_decl): Add is_auto parameter and
>      add !is_auto to condition where TREE_STATIC (decl) is set.
>      * trans-common.c (build_equiv_decl): Add local variable is_auto,
>      set it true if an atomatic attribute is encountered in the variable

atomatic? I read atomic but you mean automatic.

>      list.  Call build_equiv_decl with is_auto as an additional parameter.
>      flag_dec_format_defaults is enabled.
>      * trans-common.c (accumulate_equivalence_attributes) : New subroutine.
>      * trans-common.c (find_equivalence) : New local variable dummy_symbol,
>      accumulated equivalence attributes from each symbol then check for
>      conflicts.

I'm just curious why you don't gfc_copy_attr for the most part of accumulate_equivalence_attributes?
thanks,
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Automatics in equivalence statements

Mark Eggleston

On 25/06/2019 00:17, Jeff Law wrote:

> On 6/24/19 2:19 AM, Bernhard Reutner-Fischer wrote:
>> On Fri, 21 Jun 2019 07:10:11 -0700
>> Steve Kargl <[hidden email]> wrote:
>>
>>> On Fri, Jun 21, 2019 at 02:31:51PM +0100, Mark Eggleston wrote:
>>>> Currently variables with the AUTOMATIC attribute can not appear in an
>>>> EQUIVALENCE statement. However its counterpart, STATIC, can be used in
>>>> an EQUIVALENCE statement.
>>>>
>>>> Where there is a clear conflict in the attributes of variables in an
>>>> EQUIVALENCE statement an error message will be issued as is currently
>>>> the case.
>>>>
>>>> If there is no conflict e.g. a variable with a AUTOMATIC attribute and a
>>>> variable(s) without attributes all variables in the EQUIVALENCE will
>>>> become AUTOMATIC.
>>>>
>>>> Note: most of this patch was written by Jeff Law <[hidden email]>
>>>>
>>>> Please review.
>>>>
>>>> ChangeLogs:
>>>>
>>>> gcc/fortran
>>>>
>>>>       Jeff Law  <[hidden email]>
>>>>       Mark Eggleston  <[hidden email]>
>>>>
>>>>       * gfortran.h: Add check_conflict declaration.
>>> This is wrong.  By convention a routine that is not static
>>> has the gfc_ prefix.
>>>
>> Furthermore doesn't this export indicate that you're committing a
>> layering violation somehow?
> Possibly.  I'm the original author, but my experience in our fortran
> front-end is minimal.  I fully expected this patch to need some tweaking.
>
> We certainly don't want to recreate all the checking that's done in
> check_conflict.  We just need to defer it to a later point --
> find_equivalence seemed like a good point since we've got the full
> equivalence list handy and can accumulate the attributes across the
> entire list, then check for conflicts.
>
> If there's a concrete place where you think we should be doing this, I'm
> all ears.
>
Any suggestions will be appreciate.

>>>       * symbol.c (check_conflict): Remove automatic in equivalence conflict
>>>       check.
>>>       * symbol.c (save_symbol): Add check for in equivalence to stop the
>>>       the save attribute being added.
>>>       * trans-common.c (build_equiv_decl): Add is_auto parameter and
>>>       add !is_auto to condition where TREE_STATIC (decl) is set.
>>>       * trans-common.c (build_equiv_decl): Add local variable is_auto,
>>>       set it true if an atomatic attribute is encountered in the variable
>> atomatic? I read atomic but you mean automatic.
> Yes.
>
>>>       list.  Call build_equiv_decl with is_auto as an additional parameter.
>>>       flag_dec_format_defaults is enabled.
>>>       * trans-common.c (accumulate_equivalence_attributes) : New subroutine.
>>>       * trans-common.c (find_equivalence) : New local variable dummy_symbol,
>>>       accumulated equivalence attributes from each symbol then check for
>>>       conflicts.
>> I'm just curious why you don't gfc_copy_attr for the most part of accumulate_equivalence_attributes?
>> thanks,
> Simply didn't know about it.  It could probably significantly simplify
> the accumulation of attributes step.
Using gfc_copy_attr causes a great many "Duplicate DIMENSION attribute
specified at (1)" errors. This is because there is a great deal of
checking done instead of simply keeping track of the attributes used
which is all that is required for determining whether there is a
conflict in the equivalence statement.

Also, the final section of accumulate_equivalence_attributes involving
SAVE, INTENT and ACCESS look suspect to me. I'll check and update the
patch if necessary.

> Jeff
>
>
>
--
https://www.codethink.co.uk/privacy.html

Reply | Threaded
Open this post in threaded view
|

**ping** Re: [PATCH] Automatics in equivalence statements

Mark Eggleston
**ping**

On 01/07/2019 10:35, Mark Eggleston wrote:

>
> On 25/06/2019 14:17, Mark Eggleston wrote:
>>
>> On 25/06/2019 00:17, Jeff Law wrote:
>>> On 6/24/19 2:19 AM, Bernhard Reutner-Fischer wrote:
>>>> On Fri, 21 Jun 2019 07:10:11 -0700
>>>> Steve Kargl <[hidden email]> wrote:
>>>>
>>>>> On Fri, Jun 21, 2019 at 02:31:51PM +0100, Mark Eggleston wrote:
>>>>>> Currently variables with the AUTOMATIC attribute can not appear
>>>>>> in an
>>>>>> EQUIVALENCE statement. However its counterpart, STATIC, can be
>>>>>> used in
>>>>>> an EQUIVALENCE statement.
>>>>>>
>>>>>> Where there is a clear conflict in the attributes of variables in an
>>>>>> EQUIVALENCE statement an error message will be issued as is
>>>>>> currently
>>>>>> the case.
>>>>>>
>>>>>> If there is no conflict e.g. a variable with a AUTOMATIC
>>>>>> attribute and a
>>>>>> variable(s) without attributes all variables in the EQUIVALENCE will
>>>>>> become AUTOMATIC.
>>>>>>
>>>>>> Note: most of this patch was written by Jeff Law <[hidden email]>
>>>>>>
>>>>>> Please review.
>>>>>>
>>>>>> ChangeLogs:
>>>>>>
>>>>>> gcc/fortran
>>>>>>
>>>>>>       Jeff Law  <[hidden email]>
>>>>>>       Mark Eggleston <[hidden email]>
>>>>>>
>>>>>>       * gfortran.h: Add check_conflict declaration.
>>>>> This is wrong.  By convention a routine that is not static
>>>>> has the gfc_ prefix.
> Updated the code to use gfc_check_conflict instead.
>>>>>
>>>> Furthermore doesn't this export indicate that you're committing a
>>>> layering violation somehow?
>>> Possibly.  I'm the original author, but my experience in our fortran
>>> front-end is minimal.  I fully expected this patch to need some
>>> tweaking.
>>>
>>> We certainly don't want to recreate all the checking that's done in
>>> check_conflict.  We just need to defer it to a later point --
>>> find_equivalence seemed like a good point since we've got the full
>>> equivalence list handy and can accumulate the attributes across the
>>> entire list, then check for conflicts.
>>>
>>> If there's a concrete place where you think we should be doing this,
>>> I'm
>>> all ears.
>>>
>> Any suggestions will be appreciate.
>>>>>       * symbol.c (check_conflict): Remove automatic in equivalence
>>>>> conflict
>>>>>       check.
>>>>>       * symbol.c (save_symbol): Add check for in equivalence to
>>>>> stop the
>>>>>       the save attribute being added.
>>>>>       * trans-common.c (build_equiv_decl): Add is_auto parameter and
>>>>>       add !is_auto to condition where TREE_STATIC (decl) is set.
>>>>>       * trans-common.c (build_equiv_decl): Add local variable
>>>>> is_auto,
>>>>>       set it true if an atomatic attribute is encountered in the
>>>>> variable
>>>> atomatic? I read atomic but you mean automatic.
>>> Yes.
>>>
>>>>>       list.  Call build_equiv_decl with is_auto as an additional
>>>>> parameter.
>>>>>       flag_dec_format_defaults is enabled.
>>>>>       * trans-common.c (accumulate_equivalence_attributes) : New
>>>>> subroutine.
>>>>>       * trans-common.c (find_equivalence) : New local variable
>>>>> dummy_symbol,
>>>>>       accumulated equivalence attributes from each symbol then
>>>>> check for
>>>>>       conflicts.
>>>> I'm just curious why you don't gfc_copy_attr for the most part of
>>>> accumulate_equivalence_attributes?
>>>> thanks,
>>> Simply didn't know about it.  It could probably significantly simplify
>>> the accumulation of attributes step.
>> Using gfc_copy_attr causes a great many "Duplicate DIMENSION
>> attribute specified at (1)" errors. This is because there is a great
>> deal of checking done instead of simply keeping track of the
>> attributes used which is all that is required for determining whether
>> there is a conflict in the equivalence statement.
>>
>> Also, the final section of accumulate_equivalence_attributes
>> involving SAVE, INTENT and ACCESS look suspect to me. I'll check and
>> update the patch if necessary.
>
> No need to check intent as there is already a conflict with DUMMY and
> INTENT can only be present for dummy variables.
>
> Please find attached an updated patch. Change logs:
>
> gcc/fortran
>
>     Jeff Law  <[hidden email]>
>     Mark Eggleston  <[hidden email]>
>
>     * gfortran.h: Add gfc_check_conflict declaration.
>     * symbol.c (check_conflict): Rename cfg_check_conflict and remove
>     static.
>     * symbol.c (cfg_check_conflict): Remove automatic in equivalence
>     conflict check.
>     * symbol.c (save_symbol): Add check for in equivalence to stop the
>     the save attribute being added.
>     * trans-common.c (build_equiv_decl): Add is_auto parameter and
>     add !is_auto to condition where TREE_STATIC (decl) is set.
>     * trans-common.c (build_equiv_decl): Add local variable is_auto,
>     set it true if an atomatic attribute is encountered in the variable
>     list.  Call build_equiv_decl with is_auto as an additional parameter.
>     flag_dec_format_defaults is enabled.
>     * trans-common.c (accumulate_equivalence_attributes) : New
> subroutine.
>     * trans-common.c (find_equivalence) : New local variable
> dummy_symbol,
>     accumulated equivalence attributes from each symbol then check for
>     conflicts.
>
> gcc/testsuite
>
>     Mark Eggleston <[hidden email]>
>
>     * gfortran.dg/auto_in_equiv_1.f90: New test.
>     * gfortran.dg/auto_in_equiv_2.f90: New test.
>     * gfortran.dg/auto_in_equiv_3.f90: New test.
>
> If the updated patch is acceptable, please can someone with the
> privileges commit the patch.
>
> Mark
>
>>
>>> Jeff
>>>
>>>
>>>
--
https://www.codethink.co.uk/privacy.html


0001-Allow-automatics-in-equivalence.patch (23K) Download Attachment