[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/MCStreamer.cpp:1199
+  report_fatal_error("emitXCOFFExceptDirective is only supported on "
+                   "XCOFF targets");
+}
----------------
nit: format


================
Comment at: llvm/lib/MC/MCXCOFFStreamer.cpp:89
+                                               bool hasDebug) {
+  report_fatal_error("emitXCOFFExceptDirective not yet supported for integrated"
+                     "assembler path.");
----------------
nit: a space after "integrated" and reformat please


================
Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-trap-annotations-64bit.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
----------------
is the `utils/update_llc_test_checks.py` able to handle `not --crash llc` RUN line? I tested this, seems it is recognized as `WARNING: Skipping non-llc RUN line: not --crash llc`. If so, maybe we should delete this line as we are manually changing this file?


================
Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-trap-annotations-64bit.ll:11
+
+; OBJ: LLVM ERROR: emitXCOFFExceptDirective not yet supported for integratedassembler path.
+
----------------
nit: space between integrated and assembler


================
Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-trap-annotations.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu \
----------------
ditto.


================
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
----------------
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`


================
Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-trap-annotations.ll:13
+
+; OBJ: LLVM ERROR: emitXCOFFExceptDirective not yet supported for integratedassembler path.
+
----------------
nit: space between integrated and assembler


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132146