[PATCH] D132146: [PowerPC] XCOFF exception section support on the direct assembler path and exception language and reason code lowering from trap intrinsics

[ChenZheng via Phabricator via llvm-commits]

shchenz added inline comments.


================
Comment at: llvm/lib/MC/MCXCOFFStreamer.cpp:89
+                                               bool hasDebug) {
+  report_fatal_error("emitXCOFFExceptDirective not yet supported for integrated"
+                     "assembler path.");
----------------
pscoro wrote:
> shchenz wrote:
> > nit: a space after "integrated" and reformat please
> ran clang-format after adding the space, no formatting was performed
Nit: Seems for a long string, the usual way is to put the space at the end of first line, instead of at the start of the second line. Need format here if the space is put at the end of `integrated`


================
Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-trap-annotations.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
+; RUN:   --ppc-asm-full-reg-names -mcpu=pwr8 < %s | FileCheck %s
----------------
pscoro wrote:
> shchenz wrote:
> > Seems like we are testing 32 bit targets in this file? If so, we should use 32 bit triple, like `powerpc-unknown-linux-gnu` `powerpc-unknown-aix`
> `powerpc-unknown-aix` already existed, I added `powerpc-unknown-linux-gnu`, no changes to assembly generated.
hmm, there are still some lines testing 64 bit targets, like the first/fourth/fifth RUN lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132146