Re: [gfortran] Fix PR 20178: Implement g77 / f2c calling conventions

[Tobias Schlüter <tobias dot schlueter at physik dot uni-muenchen dot de>]

Paul Brook wrote:
> I vote for keeping the current default, and leaving -ff2c as an option. I'm 
> sure we will get complaints whatever we choose, so we may as well choose the 
> most efficient option. (assuming a sane C abi for that particular target of 
> course ;-)

Can we also agree on -fno-second-underscore as the default?

> I'd prefer it if this were fixed. Making the complier abort when we pass an 
> abi sensitive intrinsic as an actual argument is ok as a temporay measure.  
> Generating bad code is not. I guess the proper fix involves building two 
> copies of the specific library functions with different names and compilation 
> flags.

Exactly, unfortunately this means rewriting a lot of code, because most of
these intrinsics are C library functions which we currently use as-is.  Also,
this means changing intrinsic function translation to use different names
depending on the calling conventions.  Preferably in a manner which still uses
the more efficient calling convention when not passing the functions as actual
arguments.

>>+  else if (sym->attr.function)
>>+    {
>>+      /* Correctly set the result dummy argument.  */
>>+    }
> 
> 
> This looks wrong, or at best pointless and confusing.

Woops, looks like I didn't save the file after removing this :-(

>>+  /* f2c calling conventions require a scalar default real function to
>>+     return a double precision result.  Convert this back to default
>>+     real.  We only care about the cases that can happen in Fortran 77.
>>+  */
>>+  if (gfc_option.flag_f2c && sym->ts.type == BT_REAL 
>>+      && sym->ts.kind == gfc_default_real_kind && !sym->attr.pointer
>>+      && !(sym->result && (sym->result->attr.pointer
>>+                          || sym->result->attr.dimension)))
>>+    se->expr = fold_convert (gfc_get_real_type (sym->ts.kind), se->expr);
> 
> 
> 
>>   else
>>-    type = gfc_sym_type (sym);
>>+    {
>>+      type = gfc_sym_type (sym);
>>+      if (gfc_option.flag_f2c
>>+         && sym->ts.type == BT_REAL
>>+         && sym->ts.kind == gfc_default_real_kind)
>>+       {
> 
> 
> Shouldn't these use sym->attr.always_explicit?
> They are definitely incorrect. For example:

Good catch.  I'll add your example to the testsuite as well, and make sure
those ifs are testing the right things.

> I'm mildly surprised that the callee side of the return value didn't need an 
> explicit cast somewhere. I suspect there are cases where we force a 
> conversion even if the expressions should already be the correct type.

gfc_trans_scalar_assign explicitly converts the assignment:
      gfc_add_modify_expr (&block, lse->expr,
			   fold_convert (TREE_TYPE (lse->expr), rse->expr));
which is why this works.  Admittedly, this might be papering over other bugs.
 I'm currently running the testsuite with the call to fold_convert removed to
see what happens.  So far it seems to hold up.

Without the call there, we'd have to either generate the conversion in the
frontend (which I dismissed since the FE shouldn't have to deal with the ABI),
or reimplement how function results are handled, which I pondered for a while,
but then dismissed as too invasive (the basic idea being to make sym->result a
local variable in the function, and assigning its value to the correct place
before returning, be it the hidden argument(s) or the place specified by the C
ABI).

Thanks, I'll post an updated patch once I have time,
- Tobi