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

[Digger Lin via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section-debug.ll:2
+; RUN: llc -mtriple=powerpc64-unknown-aix -filetype=obj -o %t.o < %s
+; RUN: llvm-readobj --syms %t.o | FileCheck %s --check-prefix=SYMS64
+
----------------
should be add a 32bit test case?


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section-debug.ll:112
+; SYMS64-NEXT:      }
+; SYMS64-NEXT:    }
+; SYMS64-NEXT:    Symbol {
----------------
in order to simplify the test case, suggest to delete the checking symbol "Name: sub_test" from line 94-112.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section.ll:1
+; RUN: llc -O0 -mtriple=powerpc-ibm-aix-xcoff -filetype=obj -o %t.o < %s
+; RUN: llvm-readobj --exception-section %t.o | FileCheck %s --check-prefix=EXCEPT
----------------
same question as zhengchen  put :  "is the -O0 required?"


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section.ll:3
+; RUN: llvm-readobj --exception-section %t.o | FileCheck %s --check-prefix=EXCEPT
+; RUN: llc -O0 -mtriple=powerpc-ibm-aix-xcoff -filetype=obj -o %t.o < %s
+; RUN: llvm-readobj --section-headers %t.o | FileCheck %s --check-prefix=READ
----------------
I think you have generate the %t.o in line 1, I do not think we need to generate again ? same problem in the  following.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section.ll:76
+; READ-NEXT:      Type: STYP_DATA (0x40)
+; READ-NEXT:    }
+; READ-NEXT:    Section {
----------------
I do not think you need to check the section .text and .data


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section.ll:116
+; SYMS-NEXT:      }
+; SYMS-NEXT:    }
+; SYMS-NEXT:    Symbol {
----------------
delete the unrelated the symbol check for the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134195