[PATCH] libcxxabi/unwind: Add support for ARM EHABI in AddressSpace.hpp

Nick Kledzik kledzik at apple.com
Tue Jun 24 18:32:57 PDT 2014


The unwinder changes should be a different patch than the other libcxxabi changes.  Post-commit review is fine.  


Some other details I just noticed:

Are the changes to UnwindLevel1.c to cast logging parameters to long-long still needed now that ARM is no longer compiling that file?

It looks like the definition of static_assert() is commented out.

-Nick

On Jun 24, 2014, at 3:19 PM, Nico Weber <thakis at chromium.org> wrote:
> On Tue, Jun 24, 2014 at 2:18 PM, Nick Kledzik <kledzik at apple.com> wrote:
> Nico,
> 
> This looks good now.  
> 
> Cool, thanks! What does this mean in practice?
> 
> a) Check in Dana's patch on this thread, post next patch, wait for review
> b) Check in everything we have, in logical chunks, and use post-commit review where necessary?
> 
> Thanks,
> Nico
>  
> 
> -Nick
> 
> On Jun 22, 2014, at 2:02 PM, Nico Weber <thakis at chromium.org> wrote:
>> On Mon, Jun 16, 2014 at 1:15 PM, Nick Kledzik <kledzik at apple.com> wrote:
>> 
>> On Jun 16, 2014, at 10:05 AM, Nico Weber <thakis at chromium.org> wrote:
>> 
>>> Nick, do you want us to put EHABI in a separate file before upstreaming,
>> 
>> Yes.   It should be easy to de-conditionalize your current changes to UnwindLevel1.c and merge it with your Unwind-arm.c to make the new EHABI-only file.
>> 
>> Please take another look:
>> https://github.com/awong-dev/ndk/compare/upstream-llvm-ndkpatched...master#files_bucket
>> 
>> I renamed Unwind-arm.cpp to Unwind-EHABI.cpp, and moved all new code from UnwindLevel1.c to there instead. I also reformatted the 80col lines. The file still calls the unw_ functions, let me know if you want us to move off those before upstreaming.
>> 
>> > * In general, the conditionals should consider the possibility of three unwinders for arm: SJLJ, EHABI, and (someday maybe) Itanium.   For instance, the new functions in UnwindRegisters{Save|Restore}.S should be conditionalized with some EHABI conditional (not just __arm__ && !__APPLE), and in Registers.hpp, all the new EHABI specific stuff in Registers_arm should be conditionalized with ARM_EHABI.
>> 
>> Registers.hpp is currently free of preprocessor defines and relies on the linker throwing out unused functions, so I didn't do this. Are you worried about jumpTo() having different behavior? Right now, Registers_arm is only used for EHABI unwinding, so it seems waiting with these ifdefs until someone wants to add itanium unwinding for arm might be ok?
>> 
>> Thanks,
>> Nico
>> 
>> (ps: Please ignore the first change in UnwindRegistersSave.S, I think that's some merge bug. https://github.com/awong-dev/ndk/pull/152 fixes it.)
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140624/f003b5ff/attachment.html>


More information about the cfe-commits mailing list