Re: linux-armel fixes for clisp and ffcall

[Bruno H. <br...@cl...>]

Hi Ivan,

Thanks for the patches!

Sam Steingold wrote, referring to
<http://article.gmane.org/gmane.lisp.clisp.devel/21214>
> Bruno, what do you think?

Let me comment on the issues one by one.

1) Calling convention in ariarm.d.

> The main problem with CLISP was APCS-26 calling convention used in
> ariarm.d.

If this was the main problem, you could also have built clisp with
-DNO_ARI_ASM (or -DSAFETY=3), to work around the problem.

Realizing that the bignum or bigfloat performance is not an issue for most
programs - those who need solid scientific computations will do so on a
powerful CPU anyway - I would vote for one of these:
  * Introduce some preprocessor macros that expand to MOV on one side, MOVS
    on the other side, LDMFD with or without ^ at the end, etc. Similar to
    what is done in asmi386.hh.
  * Enable NO_ARI_ASM by default on ARM, through makemake.in.
Whichever is more easily feasible.

Your patch replaces a code which is correct on one platform but wrong on the
second one with a code that is correct on the second platform but wrong on the
first one. I would not want to apply this.

2) EXT:RUN-PROGRAM and /bin/sh symlink

I agree with Sam: For EXECUTE or RUN-PROGRAM, it is fine to resolve symlinks
in "/bin/" and then append "sh", but not resolve symlinks in "/bin/sh".

A --with-shell option is dangerous: if not documented properly, someone will
try passing --with-shell=/bin/ksh or --with-shell=/bin/tcsh...

3) EXT:READ-FLOAT / EXT:WRITE-FLOAT and endianness

Thanks for finding out that __ARM_EABI__ has to be used here. This patch is
good.

4) avcall/avcall-armel.c

These .c files were designed to be compiled by specific versions of gcc,
versions 2.x and 3.x. It may be that your change works fine; that needs to
be decided 1. by looking at the generated assembly code, 2. by testing.

5) cache-armel.c

The change is Linux specific, since it uses __ARM_NR_cacheflush. Again a
case of "replace a code which is correct on one platform but wrong on the
second one with a code that is correct on the second platform but wrong on the
first one". Please use #ifdef __linux__ to protect your code, and put the
old code in the #else case.

Then please update the Makefile rules so that instead of a single file
cache-armel.s there are two files cache-armel.s and cache-armel-linux.s,
the first one being unchanged and the second one generated by your code.

6) malloc()+mprotect() usage

You cite <http://linux.derkeiler.com/Mailing-Lists/Kernel/2005-10/4637.html>
but the error in that code was the assumption about the alignment of
pointers returned by malloc(). 'trampoline' and 'trampoline_r' take care
of these alignment issues. What 'trampoline' and 'trampoline_r' do is fine
according to the ultimate reference for mprotect() in POSIX:
  <http://www.opengroup.org/onlinepubs/9699919799/functions/mprotect.html>
In glibc, posix_memalign is implemented just the same way as malloc.
So, what was the original problem? You say "mprotect() didn't seem to
be working in all cases, although sometimes it did." Can you provide a test
case or describe in detail what was wrong?

7) miscompilation in trampoline.c

This is actually not a gcc bug, but a bug in trampoline.c: At one point it
does
     ((long *) function)[0] = 0xE28FC008;
and at the other point
     freelist = *(char**)freelist;
GCC's (and ISO C's) aliasing rules don't allow this. The right fix is to use
a 'union' of { long[4]; char *; }. This will prevent GCC from reordering
these instructions.

Bruno