bug-mes
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bug-mes] ARM mes on wip-arm - Other test failures


From: Jan Nieuwenhuizen
Subject: Re: [bug-mes] ARM mes on wip-arm - Other test failures
Date: Tue, 12 Mar 2019 07:25:35 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)

Danny Milosavljevic writes:

> So on the newest wip-arm (commit
> 05751c2763fed3865f6a764c4d6d0d184bcc1083), after the following patch,
> only the following test are failing with the default configuration:

Very nice.

> diff --git a/build-aux/build.sh.in b/build-aux/build.sh.in
> index 4f3891d7..c0c4cffb 100644
> --- a/build-aux/build.sh.in
> +++ b/build-aux/build.sh.in
> @@ -53,10 +53,11 @@ debug=
>  
>  CFLAGS="
>  $debug
> +-marm
>  "

Let's do something like.

diff --git a/build-aux/build.sh.in b/build-aux/build.sh.in
index 1b58ee7ec..e3120c75d 100644
--- a/build-aux/build.sh.in
+++ b/build-aux/build.sh.in
@@ -63,6 +63,12 @@ if test $mes_libc = mes; then
 "
 fi
 
+if test $mes_libc = arm; then
+    CFLAGS="$CFLAGS
+-marm
+"
+fi
+
 CPPFLAGS="
 -D HAVE_CONFIG_H=1
 -I ${srcdir}/../lib

>  
>  if test $mes_libc = mes; then
> -    CFLASG="$CFLAGS
> +    CFLAGS="$CFLAGS
>  -nostdinc
>  -nostdlib
>  -fno-builtin

And this one :-)

> diff --git a/build-aux/check.sh.in b/build-aux/check.sh.in
> index 6732910e..0979c575 100644
> --- a/build-aux/check.sh.in
> +++ b/build-aux/check.sh.in
> @@ -27,9 +27,9 @@ if $courageous; then
>      set +e
>  fi
>  
> -CFLAGS=
> +CFLAGS=-marm
>  if test $mes_libc = mes; then
> -    CFLAGS="
> +    CFLAGS="${CFLAGS}
>  -nostdinc
>  -nostdlib
>  -fno-builtin

We'll have to see, it would be nice to have this (eventually), we'll
have to teach module/mescc.scm about `-masm'.

> diff --git a/mes/module/mes/scm.mes b/mes/module/mes/scm.mes
> index 27dc960c..5505898a 100644
> --- a/mes/module/mes/scm.mes
> +++ b/mes/module/mes/scm.mes
> @@ -264,7 +264,7 @@
>    (let* ((radix (if (null? rest) 10 (car rest)))
>           (sign (if (< n 0) '(#\-) '())))
>      (let loop ((n (abs n)) (lst '()))
> -      (let* ((i (remainder n radix))
> +      (let* ((i (abs (remainder n radix)))
>               (lst (cons (integer->char (+ i (if (< i 10) (char->integer #\0)
>                                                  (- (char->integer #\a) 
> 10)))) lst))
>               (n (quotient n radix)))

I'm wondering about this one; can't we fix remainder instead?  Something
like

diff --git a/mes/module/mes/scm.mes b/mes/module/mes/scm.mes
index 27dc960c6..2e66063ca 100644
--- a/mes/module/mes/scm.mes
+++ b/mes/module/mes/scm.mes
@@ -326,7 +326,7 @@
       (apply = rest)))
 
 (define (remainder x y)
-  (- x (* (quotient x y) y)))
+  (abs (remainder n radix)))
 
 (define (even? x)
   (= 0 (remainder x 2)))

> Are any of those known-to-fail?

Some of them are, let's have a look.  Sorry they are documented so
badly.

> FAIL: lib/tests/scaffold/60-math.c [invalid argument]
> FAIL: lib/tests/stdio/70-printf-simple.c
> FAIL: lib/tests/stdio/70-printf.c

Hmm, stdarg/calling convention.  We'll probably need these to work for
tcc; and thus add a case for __arm__.

> FAIL: lib/tests/scaffold/76-pointer-arithmetic.c [has no case for __arm__]
> FAIL: lib/tests/scaffold/7k-for-each-elem.c [has no case for __arm__]
> FAIL: lib/tests/scaffold/7l-struct-any-size-array-simple.c [not sure that ARM 
> supports packed structs in the first place--newer ARM versions should, 
> though.  What do we need that for?]

Ah, that should be documentend (and cleaned-up! possibly in reverse
order :-)

All tests up to and including 60-* are needed to support a mes that can
run mescc to compile itself.

All 70- tests are (were) needed to get a heavily tcc to run.  We no
longer patch tcc that heavily.

All 80- tests are needed to get the current, lightly patched tcc to self
host.

Beyond that (90-, a0-) is for the mes c library to support bootstrapping
guix.

So, in this specific case we'll have to look what tcc does with arm.
Hmm, looking into the tcc sources, I don't quickly see any reason why
this test would test __packed__, specificly for tcc.  So, I think it was
added to make sure that gcc does not add alignment and the `sizeof'
would succeed.  I think it's safe to either remove the packed
altogether, or patch it out for arm.

> FAIL: lib/tests/scaffold/7r-sign-extend.c [a char with value -129 ? Is that 
> valid C?  In any case, more fundamental things are broken]

That does look strange...I must confess that I did not thouroughly study
sign extension and all cases; I purely went for a "do what gcc does"
(implementation-defined) test.  You've probably seen me do that
elsewhere too.

You'll appreciate that approach works probably best when trying to
bootstrap tcc; doing exactly what gcc does is really helpful.

So esp. in the case of arm, we should either disable this test
initially, or rewrite it.

Actually, that goes for all tests that gcc fails to pass, we can add a

--8<---------------cut here---------------start------------->8---
diff --git a/build-aux/check-mescc.sh b/build-aux/check-mescc.sh
index acaed5f25..f7f3530d1 100755
--- a/build-aux/check-mescc.sh
+++ b/build-aux/check-mescc.sh
@@ -245,6 +245,12 @@ lib/tests/scaffold/a0-call-trunc-int.c
 fi
 fi
 
+if test $mes_cpu = arm; then
+    XFAIL_TESTS="$XFAIL_TESTS
+...
+"
+fi
+
 if test $mes_cpu = x86; then
     XFAIL_TESTS="$XFAIL_TESTS
 lib/tests/mes/90-dtoab.c
--8<---------------cut here---------------end--------------->8---

for all tests that we don't want to fix right now, and possibly revisit
some of them later when we are targetting tcc.  WDYT?

> FAIL: lib/tests/setjmp/80-setjmp.c [not implemented for __arm__]
> FAIL: lib/tests/scaffold/85-sizeof.c [has no case for __arm__]
> FAIL: lib/tests/dirent/90-readdir.c [can open nonexistent directory]

Oops.  That may be a mes c lib readdir bug, we may need an extra check there?

> FAIL: lib/tests/io/90-stat.c [no case for __arm__]
> FAIL: lib/tests/mes/90-dtoab.c [problem with negative floating point numbers]
> FAIL: lib/tests/signal/90-signal.c [signal handler doesn't get called]
> XFAIL: lib/tests/stdio/90-sprintf.c

Probably safe to skip these for now.  We may not need them until much
later.

Thanks!
janneke

-- 
Jan Nieuwenhuizen <address@hidden> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | AvatarĀ® http://AvatarAcademy.com



reply via email to

[Prev in Thread] Current Thread [Next in Thread]