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

[Jerry DeLisle]

Jerry DeLisle wrote:
> 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.
> 
Passed all regression tests on x86-64-linux-gnu.

Jerry