[PATCH] D111703: [ARM] __cxa_end_cleanup should be called instead of _UnwindResume.

[Logan Chien via Phabricator via llvm-commits]

logan added inline comments.


================
Comment at: libcxxabi/src/cxa_exception.cpp:345
+first argument to _Unwind_Resume().  In addition, we are saving lr in order to
+align the stack to 16 bytes and lr will be used to identify the caller.
+_Unwind_Resume never return and we need to keep the origian lr so just branch to it.
----------------
Add `and its frame information` after `identify the caller`.


================
Comment at: libcxxabi/src/cxa_exception.cpp:346
+align the stack to 16 bytes and lr will be used to identify the caller.
+_Unwind_Resume never return and we need to keep the origian lr so just branch to it.
 */
----------------
s/origian/original/


================
Comment at: llvm/lib/CodeGen/DwarfEHPrepare.cpp:233
+
+    if (isTargetEHABICompatible(TargetTriple))
+      RewindName = "__cxa_end_cleanup";
----------------
I think in addition to `isTargetEHABICompatible` check, you should also check `(Pers == EHPersonality::GNU_CXX || Pers == EhPersonality::GNU_CXX_SjLj)` because only `__gxx_personality_v0` and `__gxx_personality_sj0` will call `__cxa_begin_cleanup` and push the exception object into the global stack.


================
Comment at: llvm/test/CodeGen/ARM/eh-resume.ll:29
+; EABI: __cxa_end_cleanup
+; ABI: _Unwind_Resume
----------------
Please add another test for `__gcc_personality_v0`, which should not use `__cxa_end_cleanup`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111703/new/

https://reviews.llvm.org/D111703