[PATCH] D89049: [AIX][XCOFF] print out the traceback info

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 28 00:28:05 PDT 2023


jhenderson accepted this revision.
jhenderson added a comment.

Apart from a couple of nits, LGTM!! Thanks for the patience, and sorry I wasn't able to review particularly efficiently.

Please don't land this without further sign off from @MaskRay and possibly @stephenpeckham.



================
Comment at: llvm/test/CodeGen/PowerPC/aix-emit-tracebacktable-clobber-register.ll:12
+; RUN: llvm-objdump -d --traceback-table --symbol-description %t.o | \
+; RUN:   FileCheck --match-full-lines --check-prefixes=OBJ-DIS,NO-FUNC-SEC %s
+
----------------
MaskRay wrote:
> You may want `--strict-whitespace` as well, otherwise the space formatting change in the code cannot be tested.
Did you miss this comment?


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:286
+    }
+
+    OS << '\n';
----------------
FWIW, I think the blank line that was removed here is okay to be present. I think @MaskRay was suggesting that a variable that is declared and then used in an if statement immediately afterwards should not have a blank line between it and the if statement.

A blank line after a non-trivial if block is usually a good thing, if there is code after the if statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89049



More information about the llvm-commits mailing list