diff mbox

RFC: Building a minimal libgfortran for nvptx

Message ID 54663710.7070405@codesourcery.com
State New
Headers show

Commit Message

Bernd Schmidt Nov. 14, 2014, 5:08 p.m. UTC
Hi Tobias,

> Does printf work? I thought I/O is not supported? Or does it just accept
> it for linking and drop it? I think Janne's patch has already dealt with
> the issue of stack allocation.

printf (or more accurately vprintf) is supported by ptx as a magic 
builtin function. We have a printf wrapper around vprintf in our newlib. 
What was Janne's patch?

> What I dislike a bit about the feature is that it is not clear what
> features will be supported for LIBGFOR_MINIMAL. Maybe configure.ac would
> be a good place to describe which features are included there (e.g. no
> I/O but "printf" etc.) and which aren't. That will make it easier to see
> what has to be modifed if one will add another differently bare system
> later on.

My view is that it makes no sense to generalize this at the moment when 
it is unknown what another target that uses this would look like. The 
time to revisit this is when there is another target.

> +#if 0
> +/* Initialize the runtime library.  */
>
> #if 0'ed code shouldn't be in the patch.

Ok, removed - I'll put it back if we ever support constructors.

> +@LIBGFOR_MINIMAL_FALSE@gfor_helper_src = intrinsics/associated.c \
> +@LIBGFOR_MINIMAL_FALSE@    intrinsics/abort.c intrinsics/access.c \
> +@LIBGFOR_MINIMAL_FALSE@    intrinsics/args.c \
> +@LIBGFOR_MINIMAL_FALSE@    intrinsics/bit_intrinsics.c \
> ...
> +@LIBGFOR_MINIMAL_TRUE@gfor_helper_src = intrinsics/associated.c \
> +@LIBGFOR_MINIMAL_TRUE@    intrinsics/abort.c intrinsics/args.c \
> ...
>
> Can't one write this differently, avoiding having most lines repeated
> and only a few missing from the second set?

Modified. New patch below, how does this look? Regenerated files not 
included. Testing in progress.


Bernd

Comments

Tobias Burnus Nov. 14, 2014, 9:28 p.m. UTC | #1
Hi Tobias,

Bernd Schmidt wrote:
>> Does printf work? I thought I/O is not supported? Or does it just accept
>> it for linking and drop it? I think Janne's patch has already dealt with
>> the issue of stack allocation.
>
> printf (or more accurately vprintf) is supported by ptx as a magic 
> builtin function. We have a printf wrapper around vprintf in our newlib.

And that prints to the normal screen? That makes debugging easier than I 
had hoped for.

> What was Janne's patch?

He changed stack allocation to static/heap allocation; the patch was 
committed 36h ago, broke Cesar's bootstrap and is here: 
https://gcc.gnu.org/ml/fortran/2014-11/msg00052.html


>> What I dislike a bit about the feature is that it is not clear what
>> features will be supported for LIBGFOR_MINIMAL. Maybe configure.ac would
>> be a good place to describe which features are included there (e.g. no
>> I/O but "printf" etc.) and which aren't. That will make it easier to see
>> what has to be modifed if one will add another differently bare system
>> later on.
>
> My view is that it makes no sense to generalize this at the moment 
> when it is unknown what another target that uses this would look like. 
> The time to revisit this is when there is another target.

Well, one can still add some notes what is supported and what isn't – 
not only for that hypothecial case but also when doing other changes to 
the library (adding new functions).

>> Can't one write this differently, avoiding having most lines repeated
>> and only a few missing from the second set?
>
> Modified. New patch below, how does this look? Regenerated files not 
> included. Testing in progress.

I like it more that way :-)


All in all: Okay when tesing succeeded. I still prefer some words what's 
excluded (or included) in minimal as comment in configure.ac, but the 
patch is also okay without.

Thanks,

Tobias
Janne Blomqvist Nov. 15, 2014, 6:21 a.m. UTC | #2
On Fri, Nov 14, 2014 at 11:28 PM, Tobias Burnus <burnus@net-b.de> wrote:
> Hi Tobias,
>
> Bernd Schmidt wrote:
>>>
>>> Does printf work? I thought I/O is not supported? Or does it just accept
>>> it for linking and drop it? I think Janne's patch has already dealt with
>>> the issue of stack allocation.
>>
>>
>> printf (or more accurately vprintf) is supported by ptx as a magic builtin
>> function. We have a printf wrapper around vprintf in our newlib.

So the normal stdin/out/err file descriptors are not available?

> And that prints to the normal screen? That makes debugging easier than I had
> hoped for.
>
>> What was Janne's patch?
>
>
> He changed stack allocation to static/heap allocation; the patch was
> committed 36h ago, broke Cesar's bootstrap and is here:
> https://gcc.gnu.org/ml/fortran/2014-11/msg00052.html

To be fair, most of the fixes were in the I/O library, or for various
I/O related syscall wrappers, which probably aren't of interest for
the nvptx backend. In any case, libgfortran doesn't use alloca() nor
VLA's anymore, if there were any issues related to that when doing
your patch you might want to review it  and see if you can make the
minimal version a bit more complete.

>>> What I dislike a bit about the feature is that it is not clear what
>>> features will be supported for LIBGFOR_MINIMAL. Maybe configure.ac would
>>> be a good place to describe which features are included there (e.g. no
>>> I/O but "printf" etc.) and which aren't. That will make it easier to see
>>> what has to be modifed if one will add another differently bare system
>>> later on.
>>
>>
>> My view is that it makes no sense to generalize this at the moment when it
>> is unknown what another target that uses this would look like. The time to
>> revisit this is when there is another target.

Are you committed to helping whoever will be doing the support for the
next minimal target then? Otherwise it seems a bit unfair to leave it
to them to clean up the mess?

> Well, one can still add some notes what is supported and what isn't – not
> only for that hypothecial case but also when doing other changes to the
> library (adding new functions).

Yes, something like this would be nice. Otherwise it seems really easy
to inadvertently break nvptx.
Bernd Schmidt Nov. 28, 2014, 5:39 p.m. UTC | #3
On 11/14/2014 10:28 PM, Tobias Burnus wrote:
> All in all: Okay when tesing succeeded. I still prefer some words what's
> excluded (or included) in minimal as comment in configure.ac, but the
> patch is also okay without.

I thought you meant something more than adding a comment. I've added 
this in the configure script, and committed the patch after testing:

# For GPU offloading, not everything in libfortran can be supported.
# Currently, the only target that has this problem is nvptx.  The
# following is a (partial) list of features that are unsupportable on
# this particular target:
# * Constructors
# * alloca
# * C library support for I/O, with printf as the one notable exception
# * C library support for other features such as signal, environment
#   variables, time functions

I wouldn't worry too much about breaking nvptx by accident, we can 
surely clean up any such problems should they arise.


Bernd
diff mbox

Patch

	* Makefile.am (AM_CFLAGS): Add -DLIBGFOR_MINIMAL if LIBGFOR_MINIMAL.
	(gfor_io_src, gfor_heper_src, gfor_src): Split into minimal and
	always included sources.
	* Makefile.in: Regenerate.
	* configure.ac (LIBGFOR_MINIMAL): New AM_CONDITIONAL.
	* configure: Regenerate.
	* caf/single.c (caf_runtime_error): Don't print messages if
	LIBGFOR_MINIMAL.
	* runtime/compile_options.c (fatal_error_in_progress,
	show_signal, backtrace_handler, maybe_find_addr2line): Guard with
	!defined LIBGFOR_MINIMAL.
	(set_options): Likewise for the backtrace code.
	* runtime/minimal.c: New file.

diff --git a/libgfortran/Makefile.am b/libgfortran/Makefile.am
index a058a01..31eb986 100644
--- a/libgfortran/Makefile.am
+++ b/libgfortran/Makefile.am
@@ -77,7 +77,16 @@  AM_CFLAGS += $(SECTION_FLAGS)
 AM_CFLAGS += $(IEEE_FLAGS)
 AM_FCFLAGS += $(IEEE_FLAGS)
 
+if LIBGFOR_MINIMAL
+AM_CFLAGS += -DLIBGFOR_MINIMAL
+endif
+
 gfor_io_src= \
+io/size_from_kind.c
+
+if !LIBGFOR_MINIMAL
+
+gfor_io_src+= \
 io/close.c \
 io/file_pos.c \
 io/format.c \
@@ -87,7 +96,6 @@  io/list_read.c \
 io/lock.c \
 io/open.c \
 io/read.c \
-io/size_from_kind.c \
 io/transfer.c \
 io/transfer128.c \
 io/unit.c \
@@ -95,6 +103,8 @@  io/unix.c \
 io/write.c \
 io/fbuf.c
 
+endif
+
 gfor_io_headers= \
 io/io.h \
 io/fbuf.h \
@@ -104,67 +114,73 @@  io/unix.h
 gfor_helper_src= \
 intrinsics/associated.c \
 intrinsics/abort.c \
-intrinsics/access.c \
 intrinsics/args.c \
 intrinsics/bit_intrinsics.c \
-intrinsics/c99_functions.c \
-intrinsics/chdir.c \
-intrinsics/chmod.c \
-intrinsics/clock.c \
-intrinsics/cpu_time.c \
 intrinsics/cshift0.c \
-intrinsics/ctime.c \
-intrinsics/date_and_time.c \
-intrinsics/dtime.c \
-intrinsics/env.c \
 intrinsics/eoshift0.c \
 intrinsics/eoshift2.c \
 intrinsics/erfc_scaled.c \
-intrinsics/etime.c \
-intrinsics/execute_command_line.c \
-intrinsics/exit.c \
 intrinsics/extends_type_of.c \
 intrinsics/fnum.c \
-intrinsics/gerror.c \
-intrinsics/getcwd.c \
-intrinsics/getlog.c \
-intrinsics/getXid.c \
-intrinsics/hostnm.c \
 intrinsics/ierrno.c \
 intrinsics/ishftc.c \
 intrinsics/iso_c_generated_procs.c \
 intrinsics/iso_c_binding.c \
-intrinsics/kill.c \
-intrinsics/link.c \
 intrinsics/malloc.c \
 intrinsics/mvbits.c \
 intrinsics/move_alloc.c \
 intrinsics/pack_generic.c \
-intrinsics/perror.c \
 intrinsics/selected_char_kind.c \
-intrinsics/signal.c \
 intrinsics/size.c \
-intrinsics/sleep.c \
 intrinsics/spread_generic.c \
 intrinsics/string_intrinsics.c \
-intrinsics/system.c \
 intrinsics/rand.c \
 intrinsics/random.c \
-intrinsics/rename.c \
 intrinsics/reshape_generic.c \
 intrinsics/reshape_packed.c \
 intrinsics/selected_int_kind.f90 \
 intrinsics/selected_real_kind.f90 \
+intrinsics/transpose_generic.c \
+intrinsics/unpack_generic.c \
+runtime/in_pack_generic.c \
+runtime/in_unpack_generic.c
+
+if !LIBGFOR_MINIMAL
+
+gfor_helper_src+= \
+intrinsics/access.c \
+intrinsics/c99_functions.c \
+intrinsics/chdir.c \
+intrinsics/chmod.c \
+intrinsics/clock.c \
+intrinsics/cpu_time.c \
+intrinsics/ctime.c \
+intrinsics/date_and_time.c \
+intrinsics/dtime.c \
+intrinsics/env.c \
+intrinsics/etime.c \
+intrinsics/execute_command_line.c \
+intrinsics/exit.c \
+intrinsics/gerror.c \
+intrinsics/getcwd.c \
+intrinsics/getlog.c \
+intrinsics/getXid.c \
+intrinsics/hostnm.c \
+intrinsics/kill.c \
+intrinsics/link.c \
+intrinsics/perror.c \
+intrinsics/signal.c \
+intrinsics/sleep.c \
+intrinsics/system.c \
+intrinsics/rename.c \
 intrinsics/stat.c \
 intrinsics/symlnk.c \
 intrinsics/system_clock.c \
 intrinsics/time.c \
-intrinsics/transpose_generic.c \
 intrinsics/umask.c \
-intrinsics/unlink.c \
-intrinsics/unpack_generic.c \
-runtime/in_pack_generic.c \
-runtime/in_unpack_generic.c
+intrinsics/unlink.c
+
+endif
 
 if IEEE_SUPPORT
 
@@ -182,19 +198,29 @@  gfor_ieee_src=
 endif
 
 gfor_src= \
-runtime/backtrace.c \
 runtime/bounds.c \
 runtime/compile_options.c \
+runtime/memory.c \
+runtime/string.c \
+runtime/select.c
+
+if LIBGFOR_MINIMAL
+
+gfor_src+= runtime/minimal.c
+
+else
+
+gfor_src+= \
+runtime/backtrace.c \
 runtime/convert_char.c \
 runtime/environ.c \
 runtime/error.c \
 runtime/fpu.c \
 runtime/main.c \
-runtime/memory.c \
 runtime/pause.c \
-runtime/stop.c \
-runtime/string.c \
-runtime/select.c
+runtime/stop.c
+
+endif
 
 i_all_c= \
 $(srcdir)/generated/all_l1.c \
diff --git a/libgfortran/caf/single.c b/libgfortran/caf/single.c
index e264fc5..632d172 100644
--- a/libgfortran/caf/single.c
+++ b/libgfortran/caf/single.c
@@ -48,13 +48,14 @@  caf_static_t *caf_static_list = NULL;
 static void
 caf_runtime_error (const char *message, ...)
 {
+#ifndef LIBGFOR_MINIMAL
   va_list ap;
   fprintf (stderr, "Fortran runtime error: ");
   va_start (ap, message);
   vfprintf (stderr, message, ap);
   va_end (ap);
   fprintf (stderr, "\n");
-
+#endif
   /* FIXME: Shutdown the Fortran RTL to flush the buffer.  PR 43849.  */
   exit (EXIT_FAILURE);
 }
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
index ada74e3..8ffdd2f 100644
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -189,6 +189,8 @@  AM_CONDITIONAL(LIBGFOR_USE_SYMVER, [test "x$gfortran_use_symver" != xno])
 AM_CONDITIONAL(LIBGFOR_USE_SYMVER_GNU, [test "x$gfortran_use_symver" = xgnu])
 AM_CONDITIONAL(LIBGFOR_USE_SYMVER_SUN, [test "x$gfortran_use_symver" = xsun])
 
+AM_CONDITIONAL(LIBGFOR_MINIMAL, [test "x${target_cpu}" = xnvptx])
+
 # Figure out whether the compiler supports "-ffunction-sections -fdata-sections",
 # similarly to how libstdc++ does it
 ac_test_CFLAGS="${CFLAGS+set}"
diff --git a/libgfortran/runtime/compile_options.c b/libgfortran/runtime/compile_options.c
index 748ac23..6f78f9c 100644
--- a/libgfortran/runtime/compile_options.c
+++ b/libgfortran/runtime/compile_options.c
@@ -29,7 +29,7 @@  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 /* Useful compile-time options will be stored in here.  */
 compile_options_t compile_options;
 
-
+#ifndef LIBGFOR_MINIMAL
 volatile sig_atomic_t fatal_error_in_progress = 0;
 
 
@@ -146,6 +146,7 @@  maybe_find_addr2line (void)
   if (options.backtrace == -1)
     find_addr2line ();
 }
+#endif
 
 /* Set the usual compile-time options.  */
 extern void set_options (int , int []);
@@ -176,6 +177,7 @@  set_options (int num, int options[])
   if (num >= 9)
     compile_options.fpe_summary = options[8];
 
+#ifndef LIBGFOR_MINIMAL
   /* If backtrace is required, we set signal handlers on the POSIX
      2001 signals with core action.  */
   if (compile_options.backtrace)
@@ -212,6 +214,7 @@  set_options (int num, int options[])
 
       maybe_find_addr2line ();
     }
+#endif
 }
 
 
diff --git a/libgfortran/runtime/minimal.c b/libgfortran/runtime/minimal.c
new file mode 100644
index 0000000..8dede63
--- /dev/null
+++ b/libgfortran/runtime/minimal.c
@@ -0,0 +1,158 @@ 
+/* Copyright (C) 2002-2014 Free Software Foundation, Inc.
+   Contributed by Andy Vaught and Paul Brook <paul@nowt.org>
+
+This file is part of the GNU Fortran runtime library (libgfortran).
+
+Libgfortran is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+Libgfortran is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+GNU General Public License for more details.
+
+Under Section 7 of GPL version 3, you are granted additional
+permissions described in the GCC Runtime Library Exception, version
+3.1, as published by the Free Software Foundation.
+
+You should have received a copy of the GNU General Public License and
+a copy of the GCC Runtime Library Exception along with this program;
+see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+<http://www.gnu.org/licenses/>.  */
+
+#include "libgfortran.h"
+#include <stdlib.h>
+#include <string.h>
+#include <limits.h>
+#include <errno.h>
+
+
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
+/* Stupid function to be sure the constructor is always linked in, even
+   in the case of static linking.  See PR libfortran/22298 for details.  */
+void
+stupid_function_name_for_static_linking (void)
+{
+  return;
+}
+
+options_t options;
+
+/* This will be 0 for little-endian
+   machines and 1 for big-endian machines.
+
+   Currently minimal libgfortran only runs on little-endian devices
+   which don't support constructors so this is just a constant.  */
+int big_endian = 0;
+
+static int argc_save;
+static char **argv_save;
+
+static const char *exe_path;
+
+/* recursion_check()-- It's possible for additional errors to occur
+ * during fatal error processing.  We detect this condition here and
+ * exit with code 4 immediately. */
+
+#define MAGIC 0x20DE8101
+
+static void
+recursion_check (void)
+{
+  static int magic = 0;
+
+  /* Don't even try to print something at this point */
+  if (magic == MAGIC)
+    sys_abort ();
+
+  magic = MAGIC;
+}
+
+#define STRERR_MAXSZ 256
+
+void
+os_error (const char *message)
+{
+  recursion_check ();
+  printf ("Operating system error: ");
+  printf ("%s\n", message);
+  exit (1);
+}
+iexport(os_error);
+
+void
+runtime_error (const char *message, ...)
+{
+  va_list ap;
+
+  recursion_check ();
+  printf ("Fortran runtime error: ");
+  va_start (ap, message);
+  vprintf (message, ap);
+  va_end (ap);
+  printf ("\n");
+  exit (2);
+}
+iexport(runtime_error);
+
+/* void runtime_error_at()-- These are errors associated with a
+ * run time error generated by the front end compiler.  */
+
+void
+runtime_error_at (const char *where, const char *message, ...)
+{
+  va_list ap;
+
+  recursion_check ();
+  printf ("Fortran runtime error: ");
+  va_start (ap, message);
+  vprintf (message, ap);
+  va_end (ap);
+  printf ("\n");
+  exit (2);
+}
+iexport(runtime_error_at);
+
+/* Return the full path of the executable.  */
+char *
+full_exe_path (void)
+{
+  return (char *) exe_path;
+}
+
+
+/* Set the saved values of the command line arguments.  */
+
+void
+set_args (int argc, char **argv)
+{
+  argc_save = argc;
+  argv_save = argv;
+  exe_path = argv[0];
+}
+iexport(set_args);
+
+
+/* Retrieve the saved values of the command line arguments.  */
+
+void
+get_args (int *argc, char ***argv)
+{
+  *argc = argc_save;
+  *argv = argv_save;
+}
+
+/* sys_abort()-- Terminate the program showing backtrace and dumping
+   core.  */
+
+void
+sys_abort (void)
+{
+  printf ("Abort called.\n");
+  abort();
+}