Time for PR 35150 (Bind(C)'s C_LOC; 4.3 regression)?

[Jerry DeLisle]

Jerry DeLisle wrote:
> Jerry DeLisle wrote:
>> FX Coudert wrote:
>>> Hi all,
>>>
>>> I'm answer publicly a mail by Tobias asking me if I could look into 
>>> the ISO_C_BINDING PR35150, because I have a solution but won't be in 
>>> position to test it fully or make a proper submission.
>>>
>>> To quote Tobias, "PR is a regression versus GCC 4.3.0 2007-07-16, 
>>> where using C_LOC(variable_with_SAVE) as actual argument worked". 
>>> This is rather annoying as it is not really a corner case, and we 
>>> want the first release with ISO_C_BINDING to be all shiny! I'll post 
>>> a more complete analysis in the PR soon, but I don't understand where 
>>> exactly we are failing to emit complete valid code. What I do 
>>> understand is that the result of C_LOC is stored into a variable, 
>>> which is "static void * *" instead of being "void *" (or "void *", 
>>> that doesn't matter), as was the case before. I don't know where is 
>>> the code that actually creates that variable, nor why it wants to 
>>> make it that way.
>>>
>>> Even though, I have found a workaround, which although not perfect 
>>> makes everything pass and is only a tiny missed-optimization (ie we 
>>> create one useless variable declaration, which the middle-end will 
>>> remove). It suffices to call gfc_evaluate_variable() to force the 
>>> creation of the variable right away. The patch is as follows:
>>>
>>>> Index: trans-expr.c
>>>> ===================================================================
>>>> --- trans-expr.c        (revision 132257)
>>>> +++ trans-expr.c        (working copy)
>>>> @@ -2246,7 +2246,16 @@
>>>>        if (sym->intmod_sym_id == ISOCBINDING_LOC)
>>>>         {
>>>>           if (arg->expr->rank == 0)
>>>> -           gfc_conv_expr_reference (se, arg->expr);
>>>> +           {
>>>> +             gfc_conv_expr_reference (se, arg->expr);
>>>> +
>>>> +             /* TODO -- the following two lines shouldn't be 
>>>> necessary, but
>>>> +                they're removed a bug is exposed later in the 
>>>> codepath.
>>>> +                This is workaround was thus introduced, but will 
>>>> have to be
>>>> +                removed; please see PR 35150 for details about the 
>>>> issue.  */
>>>> +             se->expr = convert (pvoid_type_node, se->expr);
>>>> +             se->expr = gfc_evaluate_now (se->expr, &se->pre);
>>>> +           }
>>>>           else
>>>>             {
>>>>               int f;
>>>
>>>
>>> As I debugged and wrote this mail in the Eurostar, I'm short on 
>>> battery and couldn't bootstrap or regtest fully. The patch builds 
>>> (--disable-bootstrap) and introduces no new failure in the 
>>> gfortran.dg/*iso*, gfortran.dg/c_*, gfortran.dg/bind* and 
>>> gfortran.dg/interface* tests (on i386-darwin). I'll be on the run for 
>>> the next few days, so if someone can help by bootstrapping it and 
>>> regtesting it. If you even want to submit, review or commit, please 
>>> feel free.
>>
>> I had to manually apply the patch.  This is probably because I have 
>> other patches pending in this file.
>>
>> I am regression testing, but will not know results until this evening 
>> (+12 hours from now).  If it passes, I will commit for you with Tobias 
>> approval.
>>
>> Tobias, if you beat me too it, please go ahead.
>>
>> Jerry
>>
> I get failures with the patch:
> 
> FAIL: gfortran.dg/data_bounds_1.f90  -O   (test for errors, line 10)
> FAIL: gfortran.dg/data_bounds_1.f90  -O   (test for errors, line 11)
> FAIL: gfortran.dg/data_bounds_1.f90  -O   (test for errors, line 14)
> FAIL: gfortran.dg/data_bounds_1.f90  -O   (test for errors, line 15)
> FAIL: gfortran.dg/volatile11.f90  -O  scan-tree-dump optimized 
> "NotOptimizedAway1"
> FAIL: gfortran.dg/volatile11.f90  -O  scan-tree-dump optimized 
> "NotOptimizedAway2"
> FAIL: gfortran.dg/where_operator_assign_4.f90  -O   (test for errors, 
> line 25)
> FAIL: gfortran.dg/where_operator_assign_4.f90  -O   (test for errors, 
> line 28)
> FAIL: gfortran.dg/where_operator_assign_4.f90  -O  (test for excess errors)

The above failures were due to some missing unrelated patches in my local tree. 
  My apologies.  Restarted regression testing.


Jerry