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

[Jerry DeLisle <jvdelisle at verizon dot net>]

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)

Jerry