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

[Hubert Tong via Phabricator via llvm-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: llvm/lib/MC/XCOFFObjectWriter.cpp:458
+  assert(isPowerOf2_32(Align) && "Alignment must be a power of 2.");
+  assert(((Sec->getCSectType() & 0xF8) == 0) && "Csect type exceeds 3-bits");
+  unsigned Log2Align = Log2_32(Align);
----------------
sfertile wrote:
> hubert.reinterpretcast wrote:
> > sfertile wrote:
> > > hubert.reinterpretcast wrote:
> > > > sfertile wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > I believe it is more robust to use `Sec->getCSectType() <= 0x07u`.
> > > > > > 
> > > > > > Also:
> > > > > > s/Csect/CSect/;
> > > > > > s/3-bits/3 bits./;
> > > > > Updated. Most of the tools and documentation use either `CSECT` or `csect`.  In these error messages 'csect' is the first word so I am capitalizing it. Should we always stick to 'CSect', and why?
> > > > Why did we name the functions `CSect`? I don't see why we need different styles for how to capitalize "csect".
> > > I honestly don't know. I've seen all 3 version used recently (and pretty sure I've used all 3 myself). If there is an 'official' way to capitalize it then lets use that, other wise lets pick one and use it consistently through out llvm codebase. I don't care which format, as long as we are consistent.
> > Agreed on consistency. Unfortunately following the "most official" way does not help with consistency. The Assembler Language Reference capitalizes as CSECT; however, my understanding of the LLVM style is that it means we should use CSECT in comments/messages, and `Csect` in code. `as` does generate at least one message where the capitalization is "Csect". In any case, "CSect" seems to be the odd one out, which is unfortunate, because then we'd have to rename functions.
> I've replaced 'CSect' with 'Csect' in all new code added in this patch, and updated any of the error messages or comments that refer to a singular csect to use 'CSECT'. I wasn't sure about how to treat the plural 'csects' though. To me it seems like we are using plain English when talking a bout multiple csects so they should not be capitalized. Let me know if `CSECTs` is more appropriate.
> 
> I can clean up any other `CSect` usage in an NFC patch.
Perhaps I was not clear. When I said capitalizes as CSECT, I was referring to the form used at the beginning of a sentence or in title case. The Assembler Language Reference uses the word, written as "csect", in other English contexts. I think we can just avoid using the plural form in places where we should be capitalizing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65159