[PATCH] D71845: [AIX][XCOFF][AIX] Fix XCOFFObjectWriter assertion failure with alignment-related gap and improve text section output testing

[Jason Liu via Phabricator via llvm-commits]

jasonliu accepted this revision.
jasonliu added a comment.

LGTM except the minor nit.



================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:442
 
+    // There could be gap (but no zero paddings) between each sections.
+    const MCSectionXCOFF *MCSec = (*Section->Groups[0])[0].MCCsect;
----------------
DiggerLin wrote:
> hubert.reinterpretcast wrote:
> > Suggestion:
> > There could be a gap (without corresponding zero padding) between sections. 
> after I discussed with Jason offline , and we decide to delete the assertion finally 
Just to clarify, I would still like to have an assertion. But Digger pointed out writing such assertion is non-trivial and the logic is complicated which might not fit in one assert statement. 
I don't want to block this patch based on missing one assertion. But if other people feels strong about it and want to suggest a way to write the assertion, please indicate.

Nit: where is the new comment?? 


================
Comment at: llvm/test/CodeGen/PowerPC/aix-return55.ll:5
+; RUN: llvm-readobj -sections %t.o | FileCheck --check-prefix=CHECKSECT %s
 
+ at strA = private unnamed_addr constant [14 x i8] c"hello world!\0A\00", align 1
----------------
DiggerLin wrote:
> jasonliu wrote:
> > daltenty wrote:
> > > nit: We might want to add a `RUN: not` case for the 64-bit so we don't forget to add it in the future
> > Please address this. I'm worried that you have forgotten this because it seems you updated the patch, but this is still not addressed. 
> these are caused by llvm-objdump -D 
You are replying to the wrong thread.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71845