This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Ping/Repost (Updated 20120127): RFA: Add Epiphany newlib & libgloss port


Quoting Mike Frysinger <vapier@gentoo.org>:

newlib/:

what is libc/sys/epiphany/e_printf.c all about ?  i don't see anyone else
implementing e_printf(), and the style in this file is pretty bad.  easier to
just punt it, and by extension, the entire libc/sys/epiphany/ subdir.

I have added a comment about its purpose. I don't think reformatting / cleaning up this file would help unless its creators would want to do that. Otherwise it just risks to introduce bugs and cause merge issues.

similarly, what's the point of
libc/machine/epiphany/machine/stdlib.h ?  seems
like that should just get dropped.

There some tests in the GCC testsuite that require random. This header file allows these test to compile.

setjmp.S could use some touch ups -- it mixes spaces and tabs for
indentation.

In attached patch.


libgloss/:

i think you've copied way more logic than needed in your
configure.in.  pretty
sure you can (and should) drop the logic related to:
      - HAVE_GNU_LD
      - HAVE_ELF
      - HAVE_ASM_POPSECTION_DIRECTIVE
      - HAVE_SECTION_ATTRIBUTES
      - libc_symbol_prefix

Dropped.


similarly, i think you've copied useless stuff into your
Makefile.in.  i'd drop
the pattern rules for .c.o, .C.o, .S.o, .s.o, and .c.s.

That's not such a good idea:


/home/amylaar/epiphany/20120207/bld-epiphany-5/./gcc/xgcc -B/home/amylaar/epiphany/20120207/bld-epiphany-5/./gcc/ -nostdinc -B/home/amylaar/epiphany/20120207/bld-epiphany-5/epiphany-elf/newlib/ -isystem /home/amylaar/epiphany/20120207/bld-epiphany-5/epiphany-elf/newlib/targ-include -isystem /home/amylaar/epiphany/20120207/srcw/newlib/libc/include -B/home/amylaar/epiphany/20120207/bld-epiphany-5/epiphany-elf/libgloss/epiphany -L/home/amylaar/epiphany/20120207/bld-epiphany-5/epiphany-elf/libgloss/libnosys -L/home/amylaar/epiphany/20120207/srcw/libgloss/epiphany -B/home/amylaar/epiphany/20120207-inst-5/epiphany-elf/bin/ -B/home/amylaar/epiphany/20120207-inst-5/epiphany-elf/lib/ -isystem /home/amylaar/epiphany/20120207-inst-5/epiphany-elf/include -isystem /home/amylaar/epiphany/20120207-inst-5/epiphany-elf/sys-include -L/home/amylaar/epiphany/20120207/bld-epiphany-5/./ld -g -O2 -c -o fstat.o ../../../../srcw/libgloss/epiphany/fstat.c
../../../../srcw/libgloss/epiphany/fstat.c:33:21: fatal error: syscall.h: No such file or directory
compilation terminated.
make[2]: *** [fstat.o] Error 1


With these rules it works:
/home/amylaar/epiphany/20120207/bld-epiphany-5/./gcc/xgcc -B/home/amylaar/epiphany/20120207/bld-epiphany-5/./gcc/ -nostdinc -B/home/amylaar/epiphany/20120207/bld-epiphany-5/epiphany-elf/newlib/ -isystem /home/amylaar/epiphany/20120207/bld-epiphany-5/epiphany-elf/newlib/targ-include -isystem /home/amylaar/epiphany/20120207/srcw/newlib/libc/include -B/home/amylaar/epiphany/20120207/bld-epiphany-5/epiphany-elf/libgloss/epiphany -L/home/amylaar/epiphany/20120207/bld-epiphany-5/epiphany-elf/libgloss/libnosys -L/home/amylaar/epiphany/20120207/srcw/libgloss/epiphany -B/home/amylaar/epiphany/20120207-inst-5/epiphany-elf/bin/ -B/home/amylaar/epiphany/20120207-inst-5/epiphany-elf/lib/ -isystem /home/amylaar/epiphany/20120207-inst-5/epiphany-elf/include -isystem /home/amylaar/epiphany/20120207-inst-5/epiphany-elf/sys-include -L/home/amylaar/epiphany/20120207/bld-epiphany-5/./ld -g -O2 -O2 -I. -I../../../../srcw/libgloss/epiphany/.. -I../../../../srcw/libgloss/epiphany/../../newlib/libc/machine/epiphany -c -g -O2 ../../../../srcw/libgloss/epiphany/fstat.c


I've removed the duplicate .S.o rule, though.

AR_FLAGS in Makefile.in should probably be "rc" instead of "qc".

Some ports use q, some r, some both... libnosys uses qc ... Whatever. Where this makes a semantic difference, there are probably other issues, but any speed advantage is likely in the noise anyway.

the install target in Makefile.in should have "|| exit $$?" added after the
`install` command to catch errors.

Added.


does that _exit.S need to be assembly ?  seems like it'd be trivial to merge
it into _exit.c and then drop the .S file.

Done.


the style in all of the C files could do with cleaning up.  please review the
GNU coding style.  i believe `indent` should help with that.

Is newlib/libgloss now supposed to follow the GNU coding style? I thought the general policy was to keep the coding style of each file as chosen by the first author ... well, at least if they have used any, and unless you do a complete rewrite. I've looked throigh some in-source and web documentation, but there doesn't seem to be any statement on that point.

There's some strong K&R showing in newlib/libm/math .

i don't think you should be adding access.c in the epiphany-specific subdir.
this would be better implemented in common code.

A number of existing ports already have their own copy. I agree that adding it to the common code makes sense, but this is really a separate issue from adding a new port using the existing infrastructure.

why do you implement _execve() and _fork() and _getpid() and _times() and
_wait() ?  libnosys already provides those stubs.

The errno handling in libnosys is somewhat dicy. Also, having separate copies in the epiphany subdir allows to add section attributes to keep I/O stuff out of the limited on-chip RAM space. Note that getpid also different, it succeeds returning 1.

in epiphany-syscalls.c, i wonder why you have to hardcode each
syscall.  can't
you do something with integer constraints like:
      asm volatile ("trap %0" : : "i"(TRAP_write));

Yes, although "n" makes more sense here.


also, does your gcc port not support naming of specific registers in
inputs/outputs ?  it's much better to write:
      asm volatile ("trap %[trapnr]"
              : "=r0"(result), "=r3"(error)
              : [trapnr] "i"(TRAP_write), "r0"(CHAN), "r1"(ADDR), "r2"(LEN));

Huh? You can name registers in clobber lists, but not for inputs / outputs the way you show above. Using local variables assigned to specific registers with an asm declaration is the method recommended in gcc/doc/extend.texi .
2012-03-02  Joern Rennecke  <joern.rennecke@embecosm.com>

libgloss:
	* epiphany/Makefile.in (AR_FLAGS): Change to rc.
	(.S.o): Remove duplicate rule.
	(_exit.o): Remove rule depending on _exit.S .
	(install): Check for error.
	* epiphany/configure: Regenerate.
	* epiphany/configure.in (HAVE_GNU_LD, HAVE_ELF): Don't define.
	(HAVE_ASM_POPSECTION_DIRECTIVE, libc_symbol_prefix): Likewise.
	* epiphany/epiphany-syscalls.c (asm_write): Use TRAP_write.
	(asm_read): Use TRAP_read.
	(asm_open): Use TRAP_open.
	(asm_exit): Use TRAP_exit.
	(asm_exit): Use TRAP_close.
	(asm_syscall): Use TRAP_other.
	(signal): Use TRAP_fail.
	* epiphany/epiphany-syscalls.h (asm_exit): Declare as noreturn.
newlib:
	* libc/machine/epiphany/setjmp.S: Consistently use tabs for leading
	whitespace.
	* libc/sys/epiphany/e_printf.c: Add comment on the purpose of this
	file.

Index: libgloss/epiphany/Makefile.in
===================================================================
RCS file: /cvs/src/src/libgloss/epiphany/Makefile.in,v
retrieving revision 1.1
diff -p -u -r1.1 Makefile.in
--- libgloss/epiphany/Makefile.in	21 Feb 2012 22:37:47 -0000	1.1
+++ libgloss/epiphany/Makefile.in	2 Mar 2012 05:32:16 -0000
@@ -95,7 +95,7 @@ INCLUDES = -I. -I$(srcdir)/.. -I$(srcdir
 # options are passed; they're passed in $(CFLAGS).
 CFLAGS_FOR_TARGET = ${MULTILIB} ${INCLUDES} ${NEWLIB_CFLAGS}
 LDFLAGS_FOR_TARGET = ${MULTILIB} ${NEWLIB_LDFLAGS}
-AR_FLAGS = qc
+AR_FLAGS = rc
 
 .c.o:
 	$(CC) $(CFLAGS_FOR_TARGET) -O2 $(INCLUDES) -c $(CFLAGS) $<
@@ -108,12 +108,6 @@ AR_FLAGS = qc
 	$(AS) $(ASFLAGS_FOR_TARGET) $(INCLUDES) $(ASFLAGS) -o $*.o $<
 
 #
-# GCC knows to run the preprocessor on .S files before it assembles them.
-#
-.S.o:
-	$(CC) $(CFLAGS_FOR_TARGET) $(INCLUDES) $(CFLAGS) -c $<
-
-#
 # this is a bogus target that'll produce an assembler from the
 # C source with the right compiler options. this is so we can
 # track down code generation or debug symbol bugs.
@@ -169,9 +163,6 @@ libepiphany.a: $(SIMOBJS)
 	${AR} ${ARFLAGS} $@ $(SIMOBJS)
 	${RANLIB} $@
 
-_exit.o: _exit.S
-	$(CC) $(CFLAGS_FOR_TARGET) $(INCLUDES) $(CFLAGS) -c $<
-
 doc:
 
 clean mostlyclean:
@@ -184,7 +175,7 @@ distclean maintainer-clean realclean: cl
 install:
 	@for outputs in ${OUTPUTS}; do\
 	 mkdir -p $(DESTDIR)$(tooldir)/lib${MULTISUBDIR}; \
-	 $(INSTALL_PROGRAM) $${outputs} $(DESTDIR)$(tooldir)/lib${MULTISUBDIR}; \
+	 $(INSTALL_PROGRAM) $${outputs} $(DESTDIR)$(tooldir)/lib${MULTISUBDIR} || exit $$?; \
 	done
 info:
 install-info:
Index: libgloss/epiphany/configure.in
===================================================================
RCS file: /cvs/src/src/libgloss/epiphany/configure.in,v
retrieving revision 1.1
diff -p -u -r1.1 configure.in
--- libgloss/epiphany/configure.in	21 Feb 2012 22:37:47 -0000	1.1
+++ libgloss/epiphany/configure.in	2 Mar 2012 05:32:16 -0000
@@ -56,43 +56,9 @@ AC_ARG_PROGRAM
 
 AC_PROG_INSTALL
 
-AC_DEFINE(HAVE_GNU_LD, 1, [Using GNU ld])
-
-# We always use ELF, define various useful associated things.
-AC_DEFINE(HAVE_ELF, 1, [Using ELF format])
-
 # Assembler directives (the assembler handles them, whether it does anything
 # useful with them is another matter...
 AC_DEFINE(HAVE_ASM_PREVIOUS_DIRECTIVE, 1, [.previous directive allowed])
-AC_DEFINE(HAVE_ASM_POPSECTION_DIRECTIVE, 1,
-          [.pushsection/.popsection directives allowed])
-
-# Section attributes in C don't currently work
-#AC_DEFINE(HAVE_SECTION_ATTRIBUTES, 1, [support for section attributes])
-
-# Sort out what the symbol prefix is (we could just fix it as '_', but let the
-# script work it out.
-AC_CACHE_CHECK([for symbol prefix], libc_symbol_prefix, [dnl
-cat > conftest.c <<\EOF
-foo () { }
-EOF
-dnl
-libc_symbol_prefix=none
-if AC_TRY_COMMAND([${CC-cc} -S conftest.c -o - | fgrep "\$foo" > /dev/null]);
-then
-  libc_symbol_prefix='$'
-else
-  if AC_TRY_COMMAND([${CC-cc} -S conftest.c -o - | fgrep "_foo" > /dev/null]);
-  then
-    libc_symbol_prefix=_
-  fi
-fi
-rm -f conftest* ])
-if test $libc_symbol_prefix != none; then
-  AC_DEFINE_UNQUOTED(__SYMBOL_PREFIX, "$libc_symbol_prefix", [symbol prefix])
-else
-  AC_DEFINE(__SYMBOL_PREFIX, "", [symbol prefix])
-fi
 
 # Standard stuff copied from libnosys. For this we'll need to an aclocal.m4
 LIB_AC_PROG_CC
Index: libgloss/epiphany/epiphany-syscalls.c
===================================================================
RCS file: /cvs/src/src/libgloss/epiphany/epiphany-syscalls.c,v
retrieving revision 1.1
diff -p -u -r1.1 epiphany-syscalls.c
--- libgloss/epiphany/epiphany-syscalls.c	21 Feb 2012 22:37:47 -0000	1.1
+++ libgloss/epiphany/epiphany-syscalls.c	2 Mar 2012 05:51:16 -0000
@@ -56,8 +56,8 @@ int __attribute__ ((section ("libgloss_e
 	register int len asm("r2") = LEN;
 	register int result asm("r0");
 	register int error asm("r3");
-	asm ("trap 0" : "=r" (result), "=r" (error) :
-	     "r" (chan), "r" (addr), "r" (len));
+	asm ("trap %[trapnr]" : "=r" (result), "=r" (error) :
+	     [trapnr] "n" (TRAP_write), "r" (chan), "r" (addr), "r" (len));
 	if (error)
 	  errno = error;
 	return result;
@@ -70,8 +70,8 @@ int __attribute__ ((section ("libgloss_e
 	register int len asm("r2") = LEN;
 	register int result asm("r0");
 	register int error asm("r3");
-	asm ("trap 1" : "=r" (result), "=r" (error) :
-	     "r" (chan), "r" (addr), "r" (len));
+	asm ("trap %[trapnr]" : "=r" (result), "=r" (error) :
+	     [trapnr] "n" (TRAP_read), "r" (chan), "r" (addr), "r" (len));
 	if (error)
 	  errno = error;
 	return result;
@@ -85,7 +85,8 @@ asm_open(const char* FILE, int FLAGS, in
 	register int flags asm("r1") = FLAGS;
 	register int result asm("r0");
 	register int error asm("r3");
-	asm ("trap 2" : "=r" (result), "=r" (error) : "r" (file), "r" (flags));
+	asm ("trap %[trapnr]" : "=r" (result), "=r" (error) :
+	     [trapnr] "n" (TRAP_open), "r" (file), "r" (flags));
 	if (error)
 	  errno = error;
 	return result;
@@ -94,7 +95,7 @@ asm_open(const char* FILE, int FLAGS, in
 void __attribute__ ((section ("libgloss_epiphany")))  asm_exit(int STATUS)
 {
 	register int status asm("r0") = STATUS;
-	asm("trap 3" :: "r" (status));
+	asm("trap %[trapnr]" :: [trapnr] "n" (TRAP_exit), "r" (status));
 }
 
 int __attribute__ ((section ("libgloss_epiphany")))  asm_close(int CHAN)
@@ -102,7 +103,8 @@ int __attribute__ ((section ("libgloss_e
 	register int chan asm("r0") = CHAN;
 	register int result asm("r0");
 	register int error asm("r3");
-	asm ("trap 6" : "=r" (result), "=r" (error) : "r" (chan));
+	asm ("trap %[trapnr]" : "=r" (result), "=r" (error) :
+	     [trapnr] "n" (TRAP_close), "r" (chan));
 	if (error)
 	  errno = error;
 	return result;
@@ -116,7 +118,8 @@ int __attribute__ ((section ("libgloss_e
 	register int result asm("r0");
 	register int subfun asm("r3") = SUBFUN;
 	register int error asm("r3");
-	asm ("trap 7" : "=r" (result), "=r" (error) :
+	asm ("trap %[trapnr]" : "=r" (result), "=r" (error) :
+	     [trapnr] "n" (TRAP_other),
 	     "r" (p1), "r" (p2), "r" (p3), "r" (subfun));
 	if (error)
 	  errno = error;
@@ -157,7 +160,7 @@ sighandler_t __attribute__ ((section ("l
 		DEFAULT_ISR_CALLBACK();
 		break;
 	case SIG_ERR :
-		asm("trap 5");
+		asm volatile ("trap %0" :: "n" (TRAP_fail));
 		break;
 	case SIG_RESET:
 		ISR_VECTOR[HW_RESET] = handler;
Index: libgloss/epiphany/epiphany-syscalls.h
===================================================================
RCS file: /cvs/src/src/libgloss/epiphany/epiphany-syscalls.h,v
retrieving revision 1.1
diff -p -u -r1.1 epiphany-syscalls.h
--- libgloss/epiphany/epiphany-syscalls.h	21 Feb 2012 22:37:47 -0000	1.1
+++ libgloss/epiphany/epiphany-syscalls.h	2 Mar 2012 05:32:16 -0000
@@ -32,7 +32,7 @@
 int asm_write (int CHAN, void* ADDR, int LEN);
 int asm_read(int CHAN, void *ADDR, int LEN);
 int asm_open(const char* FILE, int FLAGS, int MODE);
-void asm_exit(int STATUS);
+void asm_exit(int STATUS) __attribute__ ((noreturn));
 int asm_close(int CHAN);
 int asm_syscall(void *P1, void *P2, void *P3, int SUBFUN);
 
Index: newlib/libc/machine/epiphany/setjmp.S
===================================================================
RCS file: /cvs/src/src/newlib/libc/machine/epiphany/setjmp.S,v
retrieving revision 1.1
diff -p -u -r1.1 setjmp.S
--- newlib/libc/machine/epiphany/setjmp.S	21 Feb 2012 22:34:31 -0000	1.1
+++ newlib/libc/machine/epiphany/setjmp.S	2 Mar 2012 05:32:17 -0000
@@ -27,10 +27,10 @@
    POSSIBILITY OF SUCH DAMAGE.                                               */
 
 	.file "setjmp.S"
-        .section .text
-        .align 4
+	.section .text
+	.align 4
 	.global  _setjmp
-        .type _setjmp, %function
+	.type _setjmp, %function
 _setjmp:
 	strd lr,[r0]
 	strd r4,[r0,1]
@@ -62,4 +62,4 @@ _longjmp:
 	mov r0,#1
 	movne r0,r1
 	jr lr
-        .size   _longjmp, .-_longjmp
+	.size   _longjmp, .-_longjmp
Index: newlib/libc/sys/epiphany/e_printf.c
===================================================================
RCS file: /cvs/src/src/newlib/libc/sys/epiphany/e_printf.c,v
retrieving revision 1.1
diff -p -u -r1.1 e_printf.c
--- newlib/libc/sys/epiphany/e_printf.c	21 Feb 2012 22:34:31 -0000	1.1
+++ newlib/libc/sys/epiphany/e_printf.c	2 Mar 2012 05:32:17 -0000
@@ -25,6 +25,14 @@
    CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
    ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
    POSSIBILITY OF SUCH DAMAGE.                                               */
+
+/* The on-chip memory of the Epiphany hardware is too small to accomodate
+   printf, and it is often not feasible to attach large external memories
+   in a hardware bring-up situation.
+   Fortunately, gdb can be rigged to 'magically' perform all the heavy lifting
+   for a useful subset of printf when we hit a trap 7.
+   e_printf provides the interface for this emulated trap so that it can
+   be used similarly to printf.  */
 #include <stdarg.h>
 #include <string.h>
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]