diff mbox

[Fortran] PR 48174+45304: No varags if interface is known

Message ID 4D885C23.1070206@net-b.de
State New
Headers show

Commit Message

Tobias Burnus March 22, 2011, 8:21 a.m. UTC
This patch makes sure that there is no vararg if the procedure interface 
is known. Before, for functions and subroutines without arguments, no 
void_list_node.

(Related, separate and unfixed issue: For procedures without explicit 
interface, the interface should be deduced from the usage.)

Build and regtested on x86-64-linux.
OK for the trunk?

Tobias

Comments

Paul Richard Thomas March 22, 2011, 9:41 a.m. UTC | #1
Dear Tobias,

Why did Jakub's patch for PR45304 not fix this?  He visited the fix in
your patch upon build_library_function_decl_1 and
gfc_get_intrinsic_lib_fndecl.  Should
trans-decl.c(create_function_arglist) have received the same
treatment?

Otherwise it is OK for trunk.

Thanks for the patch.

Paul

On Tue, Mar 22, 2011 at 9:21 AM, Tobias Burnus <burnus@net-b.de> wrote:
> This patch makes sure that there is no vararg if the procedure interface is
> known. Before, for functions and subroutines without arguments, no
> void_list_node.
>
> (Related, separate and unfixed issue: For procedures without explicit
> interface, the interface should be deduced from the usage.)
>
> Build and regtested on x86-64-linux.
> OK for the trunk?
>
> Tobias
>
Bob Deen March 22, 2011, 4:25 p.m. UTC | #2
Hi Tobias...

I've been a lurker here since I posted a problem with varargs (stdarg, 
really) interfaces back on May 31, 2010 ("Variable argument call does 
not work on 64-bit gfortran").  You were able to determine that my 
problem existed on gfortran 4.1 but was fixed in all of 4.2 through 4.6, 
although you weren't sure how it got fixed.  We switched to 4.4 and 
everything worked fine.  I appreciate the help!!

This proposed patch is probably okay but makes me nervous.  However, the 
"related" issue scares me.  Referring to my original email, we have a 
large legacy f77 system with 15,000 calls to C stdarg functions. 
Because the stdarg and non-stdarg ABI appears to be different for x86_64 
platforms, these need to be called using the stdarg ABI.

So I hope you can understand why I might be nervous when something is 
tweaked in this area!

Our code does not declare these routines to Fortran, therefore the 
procedure interface is unknown and this patch "should" not bother 
anything.  But for the related issue... deducing the interface from 
usage is likely to break things badly in my case (and doesn't sound very 
robust in any case).  Unless I'm misunderstanding the application of the 
issue?

Would it be possible for you to try my test case again after this patch, 
just to make sure?  I can post it again if you like.

Thanks...

-Bob

Bob Deen  @  NASA-JPL Multimission Image Processing Lab
Bob.Deen@jpl.nasa.gov


On 3/22/11 1:21 AM, Tobias Burnus wrote:
> This patch makes sure that there is no vararg if the procedure interface
> is known. Before, for functions and subroutines without arguments, no
> void_list_node.
>
> (Related, separate and unfixed issue: For procedures without explicit
> interface, the interface should be deduced from the usage.)
>
> Build and regtested on x86-64-linux.
> OK for the trunk?
>
> Tobias
Tobias Burnus March 25, 2011, 5:57 p.m. UTC | #3
Dear Paul,

Paul Richard Thomas wrote:
> Why did Jakub's patch for PR45304 not fix this? He visited the fix in
> your patch upon build_library_function_decl_1 and
> gfc_get_intrinsic_lib_fndecl.

Well, my patch is in a way a follow up to Jakub's. His patch only fixed 
the library call definitions and the (in gfc_get_function_type) ensured 
that for decl with formal arguments and for the main function the void 
list is added.

Thus, no void list was added if there was no formal arglist. Well, this 
patch changes this to still add the void list, if it is known that there 
are no arguments.

> Should trans-decl.c(create_function_arglist) have received the same treatment?

I don't think so - though I might be wrong.

Committed as Rev. 171519.

Tobias
Tobias Burnus March 25, 2011, 6:21 p.m. UTC | #4
Dear Bob,

Bob Deen wrote:
> Our code does not declare these routines to Fortran, therefore the 
> procedure interface is unknown and this patch "should" not bother 
> anything.

Well, it also affects:

   SUBROUTINE foo()
   END SUBROUTINE foo
and
   REAL FUNCTION bar()
   END FUNCTION bar

that is procedures without arguments, including functions which do not 
return an argument. However, without looking at the debug output 
(DWARF), it should not be visible for the generated program - and it 
should not cause any issues.

> But for the related issue... deducing the interface from usage is 
> likely to break things badly in my case (and doesn't sound very robust 
> in any case).  Unless I'm misunderstanding the application of the issue?

I think deducing the interface should work fine; I think C does the 
same, if no prototype has been specified.

I am not sure what it means for your test case [1], thus, one probably 
should test it before. However, no patch to address that issue exists, 
yet. I have added your test program as comment to PR 44471 [2]

[1] http://gcc.gnu.org/ml/fortran/2010-05/msg00330.html
[2] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44471

Tobias
diff mbox

Patch

2011-03-22  Tobias Burnus  <burnus@net-b.de>

	PR fortran/48174
	PR fortran/45304
	* trans-types.c (gfc_get_function_type): Don't use varargs if the
	procedure is known to have no arguments.

2011-03-22  Tobias Burnus  <burnus@net-b.de>

	PR fortran/48174
	PR fortran/45304
	* gfortran.dg/ishft_4.f90: Adapt scan-tree-dump-times.
	* gfortran.dg/leadz_trailz_3.f90: Ditto

diff --git a/gcc/fortran/trans-types.c b/gcc/fortran/trans-types.c
index 258685e..8ecceea 100644
--- a/gcc/fortran/trans-types.c
+++ b/gcc/fortran/trans-types.c
@@ -2618,7 +2618,7 @@  gfc_get_function_type (gfc_symbol * sym)
 
   if (typelist)
     typelist = chainon (typelist, void_list_node);
-  else if (sym->attr.is_main_program)
+  else if (sym->attr.is_main_program || sym->attr.if_source != IFSRC_UNKNOWN)
     typelist = void_list_node;
 
   if (alternate_return)
diff --git a/gcc/testsuite/gfortran.dg/ishft_4.f90 b/gcc/testsuite/gfortran.dg/ishft_4.f90
index 0315c7f..4e2ad2b 100644
--- a/gcc/testsuite/gfortran.dg/ishft_4.f90
+++ b/gcc/testsuite/gfortran.dg/ishft_4.f90
@@ -35,6 +35,6 @@  end program
 !   -- once in the function definition itself
 !   -- plus as many times as the function is called
 !
-! { dg-final { scan-tree-dump-times "foo *\\\(\\\)" 6 "original" } }
-! { dg-final { scan-tree-dump-times "bar *\\\(\\\)" 6 "original" } }
+! { dg-final { scan-tree-dump-times "foo *\\\(\\\)" 5 "original" } }
+! { dg-final { scan-tree-dump-times "bar *\\\(\\\)" 5 "original" } }
 ! { dg-final { cleanup-tree-dump "original" } }
diff --git a/gcc/testsuite/gfortran.dg/leadz_trailz_3.f90 b/gcc/testsuite/gfortran.dg/leadz_trailz_3.f90
index f8466ff..b54a11f 100644
--- a/gcc/testsuite/gfortran.dg/leadz_trailz_3.f90
+++ b/gcc/testsuite/gfortran.dg/leadz_trailz_3.f90
@@ -26,5 +26,5 @@  end program
 !   -- once in the function definition itself
 !   -- plus as many times as the function is called
 !
-! { dg-final { scan-tree-dump-times "foo *\\\(\\\)" 8 "original" } }
+! { dg-final { scan-tree-dump-times "foo *\\\(\\\)" 7 "original" } }
 ! { dg-final { cleanup-tree-dump "original" } }