Re: a few proposed patches

[Andy Wingo <wingo@pobox.com>]

Hi Ken,

On Mon 21 May 2012 07:45, Ken Raeburn <raeburn@raeburn.org> writes:

> * Eliminate use of GC_PTR.  Looks like it's a holdover from earlier
> versions of libgc.  Some versions don't define it, so we do.  Apparently
> the 7.2 release defines it, too, which resulted in bug #11500.  It turns
> out, too, that some of the casts weren't quite right (casting to GC_PTR
> when GC_PTR* was needed), and only worked because GC_PTR is void* and
> thus can be converted to the correct type; I've tried to fix up those
> cases.  The change discards some minor abstraction of the pointer
> interface to libgc, but I don't think we really need an abstract name
> for void* anyways.
>
> * Require libgc 7.2 or better.  Too often the fix to flaky problems
> seems to be "try updating to the latest libgc and see if that fixes it",
> so let's just require it.  Or is 7.1 really *that* consistently reliable
> for our use cases on some platforms?  I decided to go with a test in the
> C code because I was having problems with include directory ordering for
> a while on my system, with both 7.1 and 7.2 installed.  A configure-time
> check would work fine in addition, but the C check takes effect after
> the various include directories for gmp, ffi, etc., are added, and using
> the order as actually determined in the makefile for compilation, which
> the configure script may or may not be consistent with.  It would be
> nice to catch a version error sooner, though.

These are related.  Until recently, the intention was that 7.1 was the
minimum version, though we supported compilation against 6.8, which is
the version in Debian stable.  As it is, the final 7.2 release was only
made a couple weeks ago, which is too new, at least for stable-2.0.

On the other hand, requiring 7.2 in master would probably be
acceptable.  Input from others is appreciated.

I think at least for stable-2.0, some more targeted fix can be
appropriate.

> * Don't use addresses of code labels with LLVM, even if the compiler
> supports them.  At least with the version of LLVM GCC on my Mac ("gcc
> version 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2336.1.00)"),

This is a very old and buggy compiler, AFAIK.  Your system might also
contain clang, which is probably better, if it works.

> the performance seems to be quite poor; "guild compile" was showing
> about a 4x penalty in CPU time.

Well, in this case it is worth making a change.  But can you try with
newer clang to see what it does?  I'd hate to turn it off for new
compilers as well.

> * Test FFI function calls with signed narrow arguments better -- both
> positive and negative values.  Currently the Mac port (with libffi
> 3.0.10) fails these tests, and I'm not sure where the bug lies.  This
> just adds more, related failures to the ones we've already got.

I don't recall what the end result was, but it could have been a bug in
a libffi compiled by that libgc.

Related:

  http://news.gmane.org/find-root.php?group=gmane.lisp.guile.bugs&article=5908
  http://news.gmane.org/find-root.php?group=gmane.lisp.guile.bugs&article=6142

> One thing still concerns me about the FFI struct size/alignment handling
> code.  It's written based on some assumptions from the SYSV ABI, and
> thus may or may not be correct for other systems not based on that ABI.
> But the tests are written in Scheme using the same assumptions; they
> should be written to test against the actual C compiler.  Otherwise we
> may wind up with FFI tests passing but FFI code not actually working.
> After the other stuff I've described above, I haven't had time to tackle
> this yet….

Good questions, these...

Andy
-- 
http://wingolog.org/