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

[Paul Scoropan via Phabricator via llvm-commits]

pscoro marked 4 inline comments as done.
pscoro added inline comments.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:797
+                     is64Bit() ? SymbolRef.getVisibilityType()
+                               : SymbolRef.getVisibilityType() | 0x0020,
+                     SymbolRef.getStorageClass(),
----------------
DiggerLin wrote:
> we emit the xcoff object code as old interpret (SymbolRef.getVisibilityType() | 0x0020). ? 
> 
>  if we want to interpret as old version what about the code in line 818.
> 
>  
> ```
> writeSymbolEntry(SymbolRef.getSymbolTableName(),
>                      CSectionRef.Address + SymbolOffset, SectionIndex,
>                      SymbolRef.getVisibilityType(),
>                      SymbolRef.getStorageClass());
> ```
We ran into an issue where this feature would not work without the 0x0020 bit set, which according to the documentation is part of the old xcoff implementation. It would make sense that the OR 0x0020 should be added to line 818 as well, but first I need to double check that my understanding is correct. Am I correct that this `void XCOFFObjectWriter::writeSymbolEntryForCsectMemberLabel` function is called with `const Symbol SymbolRef`s that are always function symbols? If this is the case, then I can add the 0x0020 to line 818 as well.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-exception-section.ll:5
+; RUN: llvm-readobj --syms %t.o | FileCheck %s --check-prefix=SYMS
+; RUN: llvm-objdump --section-headers %t.o | FileCheck %s --check-prefix=DUMP
+
----------------
DiggerLin wrote:
> is the test duplicate with
> 
> ; RUN: llvm-readobj --section-headers %t.o | FileCheck %s --check-prefix=READ ?
> 
No, the output of `llvm-readobj` and `llvm-objdump` with `--section-headers` are not the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134195