[PATCH] D100132: [libunwind][AIX] implementation of the unwinder for AIX

[Xing Xue via Phabricator via llvm-commits]

xingxue added inline comments.


================
Comment at: libunwind/include/__libunwind_config.h:65
 #  define _LIBUNWIND_CURSOR_SIZE 124
+#endif
 #  define _LIBUNWIND_HIGHEST_DWARF_REGISTER _LIBUNWIND_HIGHEST_DWARF_REGISTER_PPC
----------------
compnerd wrote:
> Should we consider just padding on clang instead of having a 1-entry difference on clang and gcc?
Currently the `libc++`/`libc++abi`/`libunwind` build scripts only support building on AIX with clang and the AIX CMake does not work well with gcc. I am deferring this change for gcc to when there is a need to build with gcc on AIX. Please let me know if you have concerns.


================
Comment at: libunwind/src/Registers.hpp:611
   void      setIP(uint32_t value) { _registers.__srr0 = value; }
+  uint64_t getCR() const { return _registers.__cr; }
+  void setCR(uint32_t value) { _registers.__cr = value; }
----------------
compnerd wrote:
> xingxue wrote:
> > cebowleratibm wrote:
> > > formatting is not consistent with surrounding code.
> > This is from the `git-clang-format`.
> Please prefer the surrounding area's formatting rather than `git-clang-format`.
Fixed, thanks!


================
Comment at: libunwind/src/Registers.hpp:1179
   void      setIP(uint64_t value) { _registers.__srr0 = value; }
+  uint64_t getCR() const { return _registers.__cr; }
+  void setCR(uint64_t value) { _registers.__cr = value; }
----------------
compnerd wrote:
> xingxue wrote:
> > cebowleratibm wrote:
> > > formatting is not consistent with surrounding code.
> > This is from `git-clang-format`.
> Similar.
Fixed, thanks!


================
Comment at: libunwind/src/UnwindCursor.hpp:1229
+    return stepWithTBTable((pint_t)this->getReg(UNW_REG_IP),
+                           (tbtable *)_info.unwind_info, _registers,
+                           _isSignalFrame);
----------------
compnerd wrote:
> Prefer a C++ style cast - `reinterpret_cast<tbtable *>(_info.unwind_info)`.
Fixed, thanks!


================
Comment at: libunwind/src/UnwindCursor.hpp:1308
+uintptr_t UnwindCursor<A, R>::getDataRelBase() {
+  return (uintptr_t)_info.extra;
+}
----------------
compnerd wrote:
> Please prefer a C++style cast: `reinterpret_cast<uintptr_t>(_info.extra);`.
Fixed, thanks!


================
Comment at: libunwind/src/UnwindCursor.hpp:1968
+bool UnwindCursor<A, R>::getInfoFromTBTable(pint_t pc, R &registers) {
+  uint32_t *p = (uint32_t *)pc;
+
----------------
compnerd wrote:
> C++ style casts please and throughout this change.
Fixed, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100132