This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [Patch, Fortran] PR 41714: [OOP] ALLOCATE SOURCE= does not properly copy the value from SOURCE
2009/10/25 Janus Weil <janus@gcc.gnu.org>:
>>>>>>> It looks OK to me except:
>>>>>>>
>>>>>>>> ? ? ? ?PR fortran/41714
>>>>>>>> ? ? ? ?* trans-expr.c (gfc_build_memcpy_call): Take care of the case that the
>>>>>>>> ? ? ? ?call to '__builtin_memcpy' is optimized away (replaced by a direct
>>>>>>>> ? ? ? ?assignment).
>>>>>>>
>>>>>>> How the heck does that work? ?It comes out as a NOP_EXPR and yet it's
>>>>>>> really an assignment..... Is that documented somewhere?
>>>>>>
>>>>>> That patch looks indeed dubious. ?It tests for an implementation detail
>>>>>> (the memcpy folder returns (void *) ({ *dst = *src; dst; })). ?You should
>>>>>> be able to unconditionally fold-convert to void_type_node as in the
>>>>>> original code. ?Instead tree_annotate_all_with_location should be fixed.
>>>>>
>>>>> Or rather the FE should not call this function - it assumes that the code
>>>>> is already gimplified.
>>>>
>>>> Ok, so you mean one should instead just do the stuff which this
>>>> function does, but without the extra checks? Like here:
>>>>
>>>> Index: gcc/fortran/trans.c
>>>> ===================================================================
>>>> --- gcc/fortran/trans.c (Revision 153538)
>>>> +++ gcc/fortran/trans.c (Arbeitskopie)
>>>> @@ -1282,7 +1282,11 @@ gfc_trans_code (gfc_code * code)
>>>> ? ? ? if (res != NULL_TREE && ! IS_EMPTY_STMT (res))
>>>> ? ? ? ?{
>>>> ? ? ? ? ?if (TREE_CODE (res) == STATEMENT_LIST)
>>>> - ? ? ? ? ? tree_annotate_all_with_location (&res, input_location);
>>>> + ? ? ? ? ? {
>>>> + ? ? ? ? ? ? tree_stmt_iterator i;
>>>> + ? ? ? ? ? ? for (i = tsi_start (res); !tsi_end_p (i); tsi_next (&i))
>>>> + ? ? ? ? ? ? ? SET_EXPR_LOCATION (tsi_stmt (i), input_location);
>>>> + ? ? ? ? ? }
>>>> ? ? ? ? ?else
>>>> ? ? ? ? ? ?SET_EXPR_LOCATION (res, input_location);
>>>
>>> No. ?I think the above should just be dropped (as well as the other
>>> call in the Fortran frontend). ?The location should have been set
>>> by the various stmt builders (like build_call_expr_loc in the
>>> memcpy case). ?For the folding of memcpy case
>>> the folder will have distributed the locations appropriately.
>>>
>>> The middle-end function can then be removed completely (the Fortran
>>> FE is the only caller). ?A patch to do so is pre-approved.
>>
>> Alright. Regtesting the attached patch now.
>
> Regtesting was not successful, unfortunately:
>
> FAIL: gfortran.dg/gomp/appendix-a/a.24.1.f90 ?-O ? (test for errors, line 20)
> FAIL: gfortran.dg/gomp/appendix-a/a.24.1.f90 ?-O ? (test for errors, line 14)
> FAIL: gfortran.dg/gomp/appendix-a/a.24.1.f90 ?-O ? (test for errors, line 20)
> FAIL: gfortran.dg/gomp/appendix-a/a.24.1.f90 ?-O ?(test for excess errors)
> FAIL: gfortran.dg/gomp/appendix-a/a.35.1.f90 ?-O ? (test for warnings, line 13)
> FAIL: gfortran.dg/gomp/appendix-a/a.35.1.f90 ?-O ?(test for excess errors)
> FAIL: gfortran.dg/gomp/appendix-a/a.35.3.f90 ?-O ? (test for warnings, line 10)
> FAIL: gfortran.dg/gomp/appendix-a/a.35.3.f90 ?-O ?(test for excess errors)
> FAIL: gfortran.dg/gomp/appendix-a/a.35.4.f90 ?-O ? (test for warnings, line 11)
> FAIL: gfortran.dg/gomp/appendix-a/a.35.4.f90 ?-O ?(test for excess errors)
> FAIL: gfortran.dg/gomp/appendix-a/a.35.6.f90 ?-O ? (test for warnings, line 9)
> FAIL: gfortran.dg/gomp/appendix-a/a.35.6.f90 ?-O ?(test for excess errors)
> FAIL: gfortran.dg/gomp/block-1.f90 ?-O ? (test for errors, line 5)
> FAIL: gfortran.dg/gomp/block-1.f90 ?-O ?(test for excess errors)
> FAIL: gfortran.dg/gomp/crayptr3.f90 ?-O ? (test for errors, line 19)
> FAIL: gfortran.dg/gomp/crayptr3.f90 ?-O ? (test for errors, line 20)
> FAIL: gfortran.dg/gomp/crayptr3.f90 ?-O ?(test for excess errors)
> FAIL: gfortran.dg/gomp/pr33439.f90 ?-O ? (test for errors, line 8)
> FAIL: gfortran.dg/gomp/pr33439.f90 ?-O ? (test for errors, line 10)
> FAIL: gfortran.dg/gomp/pr33439.f90 ?-O ? (test for errors, line 21)
> FAIL: gfortran.dg/gomp/pr33439.f90 ?-O ? (test for errors, line 22)
> FAIL: gfortran.dg/gomp/pr33439.f90 ?-O ? (test for errors, line 33)
> FAIL: gfortran.dg/gomp/pr33439.f90 ?-O ?(test for excess errors)
> FAIL: gfortran.dg/gomp/sharing-1.f90 ?-O ? (test for errors, line 12)
> FAIL: gfortran.dg/gomp/sharing-1.f90 ?-O ? (test for errors, line 24)
> FAIL: gfortran.dg/gomp/sharing-1.f90 ?-O ? (test for errors, line 25)
> FAIL: gfortran.dg/gomp/sharing-1.f90 ?-O ? (test for errors, line 26)
> FAIL: gfortran.dg/gomp/sharing-1.f90 ?-O ?(test for excess errors)
> FAIL: gfortran.dg/gomp/sharing-2.f90 ?-O ? (test for errors, line 12)
> FAIL: gfortran.dg/gomp/sharing-2.f90 ?-O ? (test for errors, line 16)
> FAIL: gfortran.dg/gomp/sharing-2.f90 ?-O ? (test for errors, line 57)
> FAIL: gfortran.dg/gomp/sharing-2.f90 ?-O ? (test for errors, line 58)
> FAIL: gfortran.dg/gomp/sharing-2.f90 ?-O ? (test for errors, line 64)
> FAIL: gfortran.dg/gomp/sharing-2.f90 ?-O ? (test for errors, line 65)
> FAIL: gfortran.dg/gomp/sharing-2.f90 ?-O ?(test for excess errors)
> FAIL: gfortran.dg/gomp/sharing-3.f90 ?-O ? (test for errors, line 28)
> FAIL: gfortran.dg/gomp/sharing-3.f90 ?-O ? (test for errors, line 30)
> FAIL: gfortran.dg/gomp/sharing-3.f90 ?-O ? (test for errors, line 33)
> FAIL: gfortran.dg/gomp/sharing-3.f90 ?-O ? (test for errors, line 34)
> FAIL: gfortran.dg/gomp/sharing-3.f90 ?-O ?(test for excess errors)
>
>
> It seems removing 'tree_annotate_all_with_location' in trans-openmp.c
> was no good idea.
Huh. I was assuming all of the above regressions were due to the hunk
in trans-openmp.c. However, it turns out they are still present after
removing it. Which means they must be due to the removal of
'tree_annotate_all_with_location' in trans.c.
So, what to do? Are we back to
Index: gcc/fortran/trans.c
===================================================================
--- gcc/fortran/trans.c (Revision 153538)
+++ gcc/fortran/trans.c (Arbeitskopie)
@@ -1282,7 +1282,11 @@ gfc_trans_code (gfc_code * code)
if (res != NULL_TREE && ! IS_EMPTY_STMT (res))
{
if (TREE_CODE (res) == STATEMENT_LIST)
- tree_annotate_all_with_location (&res, input_location);
+ {
+ tree_stmt_iterator i;
+ for (i = tsi_start (res); !tsi_end_p (i); tsi_next (&i))
+ SET_EXPR_LOCATION (tsi_stmt (i), input_location);
+ }
else
SET_EXPR_LOCATION (res, input_location);
or is there a better option? (One alternative could be to set the
location only for OpenMP cases, since all other things seem to work?)
Cheers,
Janus