<div dir="ltr">Thanks! I landed most of it in r211739, r211743 (this is the big one), r211745, 211748. I removed the long long casts from Level1.c, and omitted the static_assert bit for now. There's some smaller stuff left (Johathan Roelofs has some bare metal mode stuff, and there's a few lines with gcc compat stuff left. We'll rebase our git repo on top of what's now changed in, and then probably send out separate patches for what's left.<div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jun 24, 2014 at 6:32 PM, Nick Kledzik <span dir="ltr"><<a href="mailto:kledzik@apple.com" target="_blank">kledzik@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div style="word-wrap:break-word"><div>The unwinder changes should be a different patch than the other libcxxabi changes. Post-commit review is fine. </div><div><br></div><div><br></div><div>Some other details I just noticed:</div>
<div><br></div><div>Are the changes to UnwindLevel1.c to cast logging parameters to long-long still needed now that ARM is no longer compiling that file?</div><div><br></div><div>It looks like the definition of static_assert() is commented out.</div>
<span class=""><font color="#888888"><div><br></div><div>-Nick</div></font></span><div><div class="h5"><br><div><div>On Jun 24, 2014, at 3:19 PM, Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> wrote:</div>
<blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jun 24, 2014 at 2:18 PM, Nick Kledzik <span dir="ltr"><<a href="mailto:kledzik@apple.com" target="_blank">kledzik@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div>Nico,</div><div><br>
</div><div>This looks good now. </div></div></blockquote><div>
<br></div><div>Cool, thanks! What does this mean in practice?</div><div><br></div><div>a) Check in Dana's patch on this thread, post next patch, wait for review</div><div>b) Check in everything we have, in logical chunks, and use post-commit review where necessary?</div>
<div><br></div><div>Thanks,</div><div>Nico</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div style="word-wrap:break-word"><span><font color="#888888"><div>
<br></div><div>-Nick</div></font></span><div><div><br><div><div>On Jun 22, 2014, at 2:02 PM, Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> wrote:</div><blockquote type="cite">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Jun 16, 2014 at 1:15 PM, Nick Kledzik <span dir="ltr"><<a href="mailto:kledzik@apple.com" target="_blank">kledzik@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div>
On Jun 16, 2014, at 10:05 AM, Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> wrote:</div><br><blockquote type="cite"><div dir="ltr">Nick, do you want us to put EHABI in a separate file before upstreaming, </div>
</blockquote></div><div>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.</div></div></div></blockquote></div><br></div>
<div class="gmail_extra">Please take another look:</div><div class="gmail_extra"><a href="https://github.com/awong-dev/ndk/compare/upstream-llvm-ndkpatched...master#files_bucket" target="_blank">https://github.com/awong-dev/ndk/compare/upstream-llvm-ndkpatched...master#files_bucket</a><br>
</div><div class="gmail_extra"><br></div><div class="gmail_extra">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.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">> * 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.<br>
</div><div class="gmail_extra"><br></div><div class="gmail_extra">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?</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">Thanks,<br></div><div class="gmail_extra">Nico</div><div class="gmail_extra"><br></div><div class="gmail_extra">(ps: Please ignore the first change in UnwindRegistersSave.S, I think that's some merge bug. <a href="https://github.com/awong-dev/ndk/pull/152" target="_blank">https://github.com/awong-dev/ndk/pull/152</a> fixes it.)</div>
</div>
</blockquote></div><br></div></div></div>
</blockquote></div><br></div></div>
</blockquote></div><br></div></div></div></blockquote></div><br></div></div></div>