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

Wed Apr 13 08:53:45 PDT 2022

xingxue marked 2 inline comments as done.
xingxue added a comment.

@MaskRay @compnerd @cebowleratibm @sfertile Thanks so much for your time and effort in reviewing this patch and providing constructive suggestions to make it better!  Much appreciated!

================
Comment at: libunwind/src/assembly.h:212
+#define CSECT_ALIGN 3
+#elif defined(__ppc__)
+#define VBYTE_LEN 4
----------------
MaskRay wrote:
> Can `#elif` be changed to `#else`?
Good suggestion!  Changed in add-on commit rG9c0152cda35f.

================
Comment at: libunwind/src/config.h:63
 #else
-  #if !defined(__ELF__) && !defined(__MACH__)
-    #define _LIBUNWIND_EXPORT __declspec(dllexport)
-    #define _LIBUNWIND_HIDDEN
-  #else
-    #define _LIBUNWIND_EXPORT __attribute__((visibility("default")))
-    #define _LIBUNWIND_HIDDEN __attribute__((visibility("hidden")))
-  #endif
+#if !defined(__ELF__) && !defined(__MACH__) && !defined(_AIX)
+#define _LIBUNWIND_EXPORT __declspec(dllexport)
----------------
MaskRay wrote:
> The re-indentation seems unneeded). You may just ignore the clang-foramt diagnostic.
> 
> If we want to do formatting, it should be a separate change; I believe the problem is that .clang-format does not precisely reflect the `PPIndentWidth` the code base actually uses.
Good point, reversed the indentation in add-on commit rG9c0152cda35f.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100132