[PATCH] D65159: [PowerPC][XCOFF] Adds support for writing the .bss section for object files.

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 20 10:16:23 PDT 2019


hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

> Related to the 32-bit vs 64-bit naming and assertions: I can appreciate being defensive about this, but I think trying to disable being able to create the various types and having an assertion (or error) in every function is overkill.

Okay, I intend to continue adding comments to reviews on spots where I believe more attention than not would be needed in the future for 64-bit support, but I am fine with where we are on this patch.

The patch LGTM aside from the whole how-to-write-"csect" thing and minor comments. I think we can have these addressed on the commit.



================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:325
+    W.write<uint32_t>(Sec->Address);
+    W.write<uint32_t>(Sec->Size);
+    W.write<uint32_t>(Sec->FileOffsetToData);
----------------
hubert.reinterpretcast wrote:
> Newline before this line.
I'm not seeing the change that adds a newline between the address-writing code and the code for the next two fields.


================
Comment at: llvm/test/CodeGen/PowerPC/aix-xcoff-common.ll:8
+; RUN: not llc -mtriple powerpc64-ibm-aix-xcoff -filetype=obj -o %t.o 2>&1 \
+; RUN: < %s | FileCheck --check-prefix=64Bit %s
+
----------------
I think we've been using `64BIT` or `XCOFF64` for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65159





More information about the llvm-commits mailing list