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

[FX Coudert]

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.


FX

-- 
François-Xavier Coudert
http://www.homepages.ucl.ac.uk/~uccafco/