[PATCH] D92766: [XCOFF][AIX] Emit EH information in traceback table

Jason Liu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 7 15:17:34 PST 2020


jasonliu added inline comments.


================
Comment at: llvm/lib/BinaryFormat/XCOFF.cpp:151
 
+SmallString<32> XCOFF::getExtendedTBTableFlagString(uint8_t Flag) {
+  SmallString<32> Res;
----------------
DiggerLin wrote:
> there already have a function define getExtendedTBTableFlagString  in the  https://reviews.llvm.org/D92398
It's not in the Phabricator review that you point out. It's in D89049. But that revision is going to land after this one I think. 


================
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1994
+    if (ShouldEmitEHBlock)
+      ExtensionTableFlag |= ExtendedTBTableFlag::TB_EH_INFO;
+
----------------
DiggerLin wrote:
> since we only have ShouldEmitEHBlock.  if the ShouldEmitEHBlock is false,  it will still output an ExtensionTableFlag. Add an assert(ShouldEmitEHBlock) here ?
I think it's more natural this way. As later we might add in more flags in ExtensionTableFlag (e.g. for stack protection canary). It won't be necessarily the only field for this ExtensionTableFlag anymore. 


================
Comment at: llvm/test/CodeGen/PowerPC/aix-exception.ll:2
 ; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr4 \
-; RUN:     -mattr=-altivec < %s | \
+; RUN:     -mattr=-altivec -xcoff-traceback-table < %s | \
 ; RUN:   FileCheck --check-prefixes=ASM,ASM32 %s
----------------
DiggerLin wrote:
> we can delete -xcoff-traceback-table here after rebased on the latest https://reviews.llvm.org/D92398
For sure.


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

https://reviews.llvm.org/D92766



More information about the llvm-commits mailing list