mstorsjo added inline comments.
================ Comment at: include/unwind.h:125 uintptr_t private_2; // holds sp that phase1 found for phase2 to use -#ifndef __LP64__ +#if !defined(__LP64__) && !defined(_WIN64) // The implementation of _Unwind_Exception uses an attribute mode on the ---------------- compnerd wrote: > I think I would prefer that we do this generically as: > > #if __POINTER_WIDTH__ == 32 It seems like GCC doesn't set `__POINTER_WIDTH__`, but both GCC and clang set `__SIZEOF_POINTER__`, so I can use that. ================ Comment at: src/AddressSpace.hpp:145 public: -#ifdef __LP64__ +#if defined(__LP64__) || defined(_WIN64) typedef uint64_t pint_t; ---------------- compnerd wrote: > I think I prefer the generic: > > #if __POINTER_WIDTH__ == 64 This one probably could be simplified even further by just using `intptr_t` and `uintptr_t`, right? ================ Comment at: src/UnwindRegistersRestore.S:68 # +#if defined(_WIN32) +# On entry, thread_state pointer is in rcx ---------------- compnerd wrote: > This is confusing. Why is this `_WIN32`? Shouldn't this be `_WIN64`? I guess I could use that as well. I just used `_WIN32` out of habit as generic identifier for windows-in-whatever-form; both `_WIN32` and `_WIN64` are defined, and we're within an `#ifdef __x86_64__` anyway. I can change it into `_WIN64` for clarity. ================ Comment at: src/UnwindRegistersRestore.S:72 + movq 56(%rcx), %rax # rax holds new stack pointer + subq $16, %rax + movq %rax, 56(%rcx) ---------------- compnerd wrote: > Hmm, why is this `$16`? The `$rsp` was adjusted by `$8` in the `setjmp`. This is the exact same thing as is done right now for x86_64 on unixy systems already, just with different registers. The adjustment by 16 bytes is because we store `rcx` and `rip` on the stack here to restore them slightly differently than the others. See the `store new rcx/rip on new stack`, `restore rcx later` and `rcx was saved here earlier` and `rip was saved here` comments below. ================ Comment at: src/UnwindRegistersSave.S:66 DEFINE_LIBUNWIND_FUNCTION(unw_getcontext) +#if defined(_WIN32) + movq %rax, (%rcx) ---------------- compnerd wrote: > Shouldn't this be `_WIN64`? Same as the otherone; we're within `#ifdef __x86_64__`, but I can change it into `_WIN64` for clarity. https://reviews.llvm.org/D38819 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
