Adds support for generating the .data section in assembly files for global variables with a non-zero initialization. The support for writing the .data section in XCOFF object files will be added in a follow-on patch. Any relocations are not included in this patch.
|165 ↗||(On Diff #216404)|
This does not need to be a non-static member function. It can be a static member function.
|1122 ↗||(On Diff #216404)|
Given the code here, the name of the function (and the variable) is misleading. It should say useDotAlignForAlignment, because we don't fall back to anything else when the requested alignment is not a power of two.
|1123 ↗||(On Diff #216404)|
With the suggestion to change the error text below, I think we can remove the comment.
|1125 ↗||(On Diff #216404)|
Suggestion: "Only power-of-two alignments are supported with .align."
|33 ↗||(On Diff #216404)|
What we should write here, and whether llvm_unreachable is appropriate depends. I think that the current patch allows us to say that we do not handle switching to data sections with other storage mapping classes yet. That can be an assert, but probably not an unreachable.
assert(getMappingClass() == XCOFF::XMC_RW && "Unhandled storage-mapping class for data section.");
|1662 ↗||(On Diff #216404)|
Stray change from isBSSLocal to isBSS. Please fix.
|1663 ↗||(On Diff #216404)|
Suggestion: "Encountered a global variable kind that is not supported yet."
|25 ↗||(On Diff #216404)|
We should seek to understand why GCC and XL uses 4-byte alignment.
- Rename function useDotAlignForPowerOfTwoAlignment() to useDotAlignForAlignment() and the associated variable from UseDotAlignForPowerOfTwoAlignment to UseDotAlignForAlignment.
- Define AsmPrinter::getGVAlignmentLog2() as a static member function.
- Change from calling llvm_unreachable() to assert() as suggested for unsupported storage-mapping class of the data section.
- Fix the condition of supported global variable kind.
A few small changes, but otherwise LGTM.
|168 ↗||(On Diff #216660)|
Suggestion: "True if .align is to be used for alignment. Only power-of-two alignment is supported."
|1677 ↗||(On Diff #216660)|
Add a newline before the comment.
|1678 ↗||(On Diff #216660)|
I think this should be isBSSLocal.