On Jun 6, 2011, at 9:14 PM, Howard Hinnant wrote:

> Sorry, somehow this got by me.  I've got it marked for review now.
> 
> Howard
> 
> On Jun 6, 2011, at 8:12 PM, Nick Lewycky wrote:
> 
>> Ping! Howard, I'd like you to look at this for style.
>> 
>> Nick
>> 
>> On 3 June 2011 01:48, Nick Lewycky <[email protected]> wrote:
>> On 3 June 2011 01:43, Renato Golin <[email protected]> wrote:
>> On 3 June 2011 08:46, Nick Lewycky <[email protected]> wrote:
>>>> The attached patch implements the ARM EABI versions of
>>>> __cxa_guard_{acquire,release,abort} while trying to share as much code as
>>>> possible. It appears to me that it's unlikely that this will build 
>>>> correctly
>>>> on __APPLE__ machines, but that's beyond my ability to deal with here. I
>>>> have also not actually tested this on an ARM machine, but I did #define
>>>> LIBCXXABI_ARMEABI and build the tests and those pass on x86-64.
>> 
>> Hi Nick,
>> 
>> I also can't tell you about the Apple part (or the get_lock), but the
>> ARM __cxa_* look correct.
>> 
>> Thanks for the fast review!
>> 
>> Is there a test for this in Clang? Have you tried at least running them on 
>> OSX?
>> 
>> There's a libcxxabi/test/test_guard.cpp, but it doesn't launch any threads. 
>> I have not even built it on OSX.

Sorry for the delay in the review.

There's one bug/type-o on line 185 of cxa_guard.cpp:

            result = !is_initialized();

should be:

            result = !is_initialized(guard_object);

With that, it passes all tests I have for desktop OS X.  I can not test this on 
ARM either, but this seems like a good way to share code and thus reduce the 
possibility of bugs.

Please commit with that one fix.

Thanks,
Howard

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to