[Top][All Lists]

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

a few proposed patches

From: Ken Raeburn
Subject: a few proposed patches
Date: Mon, 21 May 2012 01:45:50 -0400

After reading the "dynamic ffi and C struct" thread this weekend, I started 
thinking, "I wonder if that's really done so as to handle X and Y and Z, and if 
we're actually testing it well enough", and got the urge to do another Mac 
build, which I hadn't done in a while.  After installing libgc 7.2 to get 
around flaky failures, and fighting with the build system a bit (I have gmp 
installed via Macports, and that tree also has libgc 7.1…), and waiting hours 
for builds to finish and looking into why, I have a few patches to propose.  
I've uploaded them to the branch wip-raeburn-misc for review, since I'm not 
sure you'll want to address the issues the same way.

* 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.

* 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)"), the performance 
seems to be quite poor; "guild compile" was showing about a 4x penalty in CPU 
time.  (For psyntax-pp.go, it was 10 minutes of CPU time vs 46 minutes.)  
Later/future versions may do better, so we can update it with version-number 
checks, unless we want to build performance tests into the configure script, 
which doesn't sound like a great idea to me.  Related to this, I made two more 
changes: Always define statement labels in the VM code anyway, because 
vm-i-scheme.c uses some of them unconditionally (which makes me wonder if any 
non-GCC configs are getting tested); and report the time taken for each "guild 
compile" command during the build.

* 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.

* 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.

Let me know if (some of?) these look good and I'll merge them to master, unless 
someone else wants to cherry-pick some of the changes first.

I've also checked in on master a couple pretty straightforward-looking fixes.  
I don't know if either would be applicable to the current release.

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….


reply via email to

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