[PATCH] D134195: [PowerPC] XCOFF exception section support on the integrated assembler path

ChenZheng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 31 18:19:25 PDT 2022


shchenz added a comment.

looks almost good to me. Just some nits left.

Please also make sure all @DiggerLin 's comments be addressed.



================
Comment at: llvm/include/llvm/MC/MCObjectWriter.h:111
+                                 unsigned FunctionSize, bool hasDebug) {
+    report_fatal_error("addExceptionEntry is only supported on "
+                       "XCOFF targets");
----------------
pscoro wrote:
> shchenz wrote:
> > Format here seems strange...
> I ran clang-format on these lines and no changes were made
clang-format can not merge the string in different lines to one line? I don't think we need two lines here


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:1202
+          bool hasExceptEntry = false;
+	  auto Entry = ExceptionSection.ExceptionTable.find(Sym.MCSym);
+          if(Entry != ExceptionSection.ExceptionTable.end()) {
----------------
nit: alignment here


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section-debug.ll:4
+; Exception auxilliary entries are present in the 64-bit tests because 64-bit && debug enabled are the requirements.
+; RUN: llc -O0 -mtriple=powerpc-ibm-aix-xcoff -filetype=obj -o %t_32.o < %s
+; RUN: llvm-readobj --syms %t_32.o | FileCheck %s --check-prefix=SYMS32
----------------
do we still need `-O0`?


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section.ll:3
+; entries should be produced as no debug information is specified.
+; RUN: llc -O0 -mtriple=powerpc-ibm-aix-xcoff -filetype=obj -o %t_32.o < %s
+; RUN: llvm-readobj --exception-section %t_32.o | FileCheck %s --check-prefix=EXCEPT
----------------
Ditto


================
Comment at: llvm/test/CodeGen/PowerPC/builtins-ppc-xlcompat-trap-annotations-td.ll:7
 ; RUN:   --ppc-asm-full-reg-names -mcpu=pwr8 < %s | FileCheck %s -check-prefix=AIX
-; RUN: not --crash llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
-; RUN:   --ppc-asm-full-reg-names -mcpu=pwr8 --filetype=obj -o /dev/null %s 2>&1 | FileCheck %s -check-prefix=OBJ
----------------
shchenz wrote:
> We should not remove the RUN line as it is the only one test for -filetype=obj. Maybe we can modify the check to a simple check, for example, `; OBJ-LABEL: test__trapd_annotation:` to check that the case does not crash in obj mode now?
This comment seems not be addressed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134195



More information about the llvm-commits mailing list