[AIX][XCOFF]Implement mergeable const

[DiggerLin]

Summary

In this patch, we map mergeable const objects to the read-only section in the same manner as const objects that are not mergeable.

[hubert.reinterpretcast]

llvm/include/llvm/MC/MCObjectFileInfo.h

176 ↗

(On Diff #234065)

Typo: s/pecific/specific/;

This comment is misleading though. XCOFF does not define such sections, and the linker does not indicate it has support for the implied merging functionality.

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1882

This code is in an XCOFF-specific function. Is there a case where the pointer remains null? If there is, I suggest to test that case. If there isn't, I suggest to assert that the pointer is non-null instead of doing a null check.

This comment also applies to the next handful of lines.

1891

If we believe that there should be special mergeable const sections, then we should assert here that we have handled all mergeable const section variants (in case additional ones are added in the future).

llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll

2

Period at the end of the sentence.

[jasonliu]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1891

Questions regarding the design

1. Do we really need special mergeable const section on AIX? What's the advantage of that? What's wrong if we just dump all of them to ReadOnlySection?

For the same C source

typedef struct {
 unsigned short id;
 unsigned char  options;
} Merge_cnst4;

int main() {
 Merge_cnst4 const cnst4  = { .id = 123};
}

Looks like GCC and xlc are able to just store the data member directly without making csects, which is more efficient. I guess clang's FE could generate better IR later to match that?

[hubert.reinterpretcast]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1870

As @jasonliu mentioned, there is not much precedent for this in the context of AIX. What is the rationale for not using the normal read-only data section?

1882

!Kind.isMergeableConst() is always true here now. There should be no need to check that here.

llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll

2

This is also testing the object file generation?

3

Should the assembly path be tested for 64-bit as well?

4

Why pwr9?

6

There's two spaces after the colon here (but one space after the colon on the previous lines).

13

The highest-alignment-first ordering here is less interesting than the reverse order in terms of how padding is generated.

95

Shouldn't this be based on Index?

[DiggerLin]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1870

I implemented the Mergeable const as gcc did.
1 .gcc use a separate csect for all mergeable const. gcc generate is as

.csect _mergeableconst.ro_[RO],4
      .align 3

cnst32:

.long   1073741824
.long   50
.space 24
.globl cnst16
.align 3

cnst16:

.long   1073741824
.long   22
.space 8
.globl cnst8
.align 2

cnst8:

.long   1073741832
.space 4
.globl cnst4
.align 1

cnst4:

.short  16392
.space 2
.csect .text[PR]

_section_.text:

.csect .data[RW],4
.long _section_.text

2. xlc use separate csect const32, const16,const8, const4 for mergeable const32 , mergeable const16, mergeable const8. mergeable const4.

.csect  cnst32{RO}, 3
 .long   0x40000000              # "@\0\0\0"
 .long   0x00000032              # "\0\0\0002"
 .long   0x00000000              # "\0\0\0\0"
 .long   0x00000000              # "\0\0\0\0"
 .long   0x00000000              # "\0\0\0\0"
 .long   0x00000000              # "\0\0\0\0"
 .long   0x00000000              # "\0\0\0\0"
 .long   0x00000000              # "\0\0\0\0"

 .csect  cnst16{RO}, 3
 .long   0x40000000              # "@\0\0\0"
 .long   0x00000016              # "\0\0\0\026"
 .long   0x00000000              # "\0\0\0\0"
 .long   0x00000000              # "\0\0\0\0"

 .csect  cnst8{RO}, 3
 .long   0x40000008              # "@\0\0\b"
 .long   0x00000000              # "\0\0\0\0"

 .csect  cnst4{RO}
 .long   0x40080000              # "@\b\0\0"

1882

yes, we already deal with Kind.isMergeableConst() before the condition. I will delete it.

llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll

3

added

4

changed to pwr4

6

changed

[DiggerLin]

address comment

[hubert.reinterpretcast]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1870

In the assembly you list, the naming of the labels (for the GCC case) or csects (for the XL case) is suspect (they are indicative of declared variables). Declared variables are not mergeable in the general sense.

It is observed that GCC on AIX uses .ro and .rw as suffixes for read-only sections; however, the naming does not seem to indicate "mergeable" as a factor.

llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll

3

I believe we always use all-caps for XCOFF.

[DiggerLin]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1870

yes, you are correct, the for gcc and xlc in the aix , I used the source code as

typedef struct {
unsigned long long id;
unsigned int option1;
unsigned long long option2;
unsigned int option3;
} Merge_cnst32;

typedef struct {
unsigned long long id;
unsigned int options;
} Merge_cnst16;

typedef struct {
unsigned int id;
unsigned int options;
} Merge_cnst8;

typedef struct {
unsigned short id;
unsigned char options;
} Merge_cnst4;

int main() {
Merge_cnst32 const cnst32 = { .id = 0x4000000000000032ULL };
Merge_cnst16 const cnst16 = { .id = 0x4000000000000016ULL };
Merge_cnst8 const cnst8 = { .id = 0x40000008 };
Merge_cnst4 const cnst4 = { .id = 0x4008 };
}

if I uncomment the int main() { , gcc and xlc will not generate any csect for the cnst32 ,cnst16,cnst8. cnst32 or they also do not put the value in any csect. it put the value in the text asm instruction directly.

1870

and in the linux , the llvm put the mergeabe const in the readonly as

.csect .rodata.cst[RO]
.align  4

.L__const.main.cnst32:

.llong  4611686018427387954     # 0x4000000000000032
.long   0                       # 0x0
.space  4
.llong  0                       # 0x0
.long   0                       # 0x0
.space  4
.align  3

.L__const.main.cnst16:

.llong  4611686018427387926     # 0x4000000000000016
.long   0                       # 0x0
.space  4
.align  3

.L__const.main.cnst8:

.long   1073741832              # 0x40000008
.long   0                       # 0x0
.align  3

.L__const.main.cnst4:

.short  16392                   # 0x4008
.byte   0                       # 0x0
.space  1

it also put the merge const in the a new .csect .rodata.cst[RO] .

[hubert.reinterpretcast]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1870

The description of the Linux behaviour seems strange to me. I would expect a .section directive.

In any case, it seems we have established that neither XL nor GCC on AIX produce special mergeable-const sections. We should either create the sections as LLVM does for ELF (knowing that it really does not do any merging), or we should just treat the object as being plain read-only). For the strings, we went with the create-like-for-ELF route for now (which has no negative effect that I know of). For the mergeable-const cases, it seems we save on a sizeable amount of code if we just treat them as plain read-only.

[DiggerLin]

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1870

Sorry. post the wrong information for the linux elf. it should be

.type   .L__const.main.cnst32,@object # @__const.main.cnst32
 .section        .rodata.cst32,"aM",@progbits,32
 .p2align        4

.L__const.main.cnst32:

.long   1073741824              # 0x4000000000000032
.long   50
.long   0                       # 0x0
.space  4
.long   0                       # 0x0
.long   0
.long   0                       # 0x0
.space  4
.size   .L__const.main.cnst32, 32

.type   .L__const.main.cnst16,@object # @__const.main.cnst16
.section        .rodata.cst16,"aM",@progbits,16
.p2align        3

.L__const.main.cnst16:

.long   1073741824              # 0x4000000000000016
.long   22
.long   0                       # 0x0
.space  4
.size   .L__const.main.cnst16, 16

.type   .L__const.main.cnst8,@object # @__const.main.cnst8
.section        .rodata.cst8,"aM",@progbits,8
.p2align        3

.L__const.main.cnst8:

.long   1073741832              # 0x40000008
.long   0                       # 0x0
.size   .L__const.main.cnst8, 8

.type   .L__const.main.cnst4,@object # @__const.main.cnst4
.section        .rodata.cst4,"aM",@progbits,4
.p2align        3

.L__const.main.cnst4:

.short  16392                   # 0x4008
.byte   0                       # 0x0
.space  1
.size   .L__const.main.cnst4, 4

.section        ".note.GNU-stack","",@progbits

[DiggerLin]

put the mergeableconst in the plain readonly section.

[hubert.reinterpretcast]

LGTM.

llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll

27

I can't say that I fully understand the choice of alignment values, but they are reasonable.

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

if (Kind.isMergeableCString()) {
     Name = SizeSpec + utostr(Align);
 
     return getContext().getXCOFFSection(
         Name, XCOFF::XMC_RO, XCOFF::XTY_SD,
         TargetLoweringObjectFileXCOFF::getStorageClassForGlobal(GO),
         Kind, /* BeginSymbolName */ nullptr);
   }
 
   if (Kind.isText())
     return TextSection;
 
   if (Kind.isData())
     return DataSection;
 
   // Zero initialized data must be emitted to the .data section because external
   // linkage control sections that get mapped to the .bss section will be linked
   // as tentative defintions, which is only appropriate for SectionKind::Common.
   if (Kind.isBSS())
     return DataSection;
 
-  if (Kind.isReadOnly() && !Kind.isMergeableConst())
+  if (Kind.isReadOnly())
     return ReadOnlySection;
 
   report_fatal_error("XCOFF other section types not yet implemented.");
 }
 
 MCSection *TargetLoweringObjectFileXCOFF::getSectionForJumpTable(
     const Function &F, const TargetMachine &TM) const {
   assert (!TM.getFunctionSections() && "Unique sections not supported on XCOFF"
           " yet.");
   assert (!F.getComdat() && "Comdat not supported on XCOFF.");
   //TODO: Enable emiting jump table to unique sections when we support it.
   return ReadOnlySection;
 }
 
 bool TargetLoweringObjectFileXCOFF::shouldPutJumpTableInFunctionSection(
     bool UsesLabelDifference, const Function &F) const {
   return false;

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

void PPCAIXAsmPrinter::EmitGlobalVariable(const GlobalVariable *GV) {
   // Create the symbol, set its storage class.
   MCSymbolXCOFF *GVSym = cast<MCSymbolXCOFF>(getSymbol(GV));
   GVSym->setStorageClass(
       TargetLoweringObjectFileXCOFF::getStorageClassForGlobal(GV));
 
   SectionKind GVKind = getObjFileLowering().getKindForGlobal(GV, TM);
   if ((!GVKind.isCommon() && !GVKind.isBSS() && !GVKind.isData() &&
        !GVKind.isReadOnly()) ||
-      GVKind.isMergeable2ByteCString() || GVKind.isMergeable4ByteCString() ||
-      GVKind.isMergeableConst())
+      GVKind.isMergeable2ByteCString() || GVKind.isMergeable4ByteCString())
     report_fatal_error("Encountered a global variable kind that is "
                        "not supported yet.");
 
   // Create the containing csect and switch to it.
   MCSectionXCOFF *Csect = cast<MCSectionXCOFF>(
       getObjFileLowering().SectionForGlobal(GV, GVKind, TM));
   OutStreamer->SwitchSection(Csect);
   GVSym->setContainingCsect(Csect);

llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll

+; This file tests the codegen of mergeable const in AIX assembly.
+; This file also tests mergeable const in XCOFF object file generation.
+; RUN: llc -verify-machineinstrs -mcpu=pwr4 -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mcpu=pwr4 -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mcpu=pwr4 -mtriple powerpc-ibm-aix-xcoff -filetype=obj -o %t.o < %s
+; RUN: llvm-objdump -D %t.o | FileCheck --check-prefix=CHECKOBJ %s
+; RUN: llvm-readobj -syms %t.o | FileCheck --check-prefix=CHECKSYM %s
+
+%struct.Merge_cnst32 = type { i64, i32, i64, i32 }
+%struct.Merge_cnst16 = type { i64, i32 }
+%struct.Merge_cnst8 = type { i32, i32 }
+%struct.Merge_cnst4 = type { i16, i8 }
+
+@__const.main.cnst32 = private unnamed_addr constant %struct.Merge_cnst32 { i64 4611686018427387954, i32 0, i64 0, i32 0 }
+@__const.main.cnst16 = private unnamed_addr constant %struct.Merge_cnst16 { i64 4611686018427387926, i32 0 }
+@__const.main.cnst8 = private unnamed_addr constant %struct.Merge_cnst8 { i32 1073741832, i32 0 }
+@__const.main.cnst4 = private unnamed_addr constant %struct.Merge_cnst4 { i16 16392, i8 0 }
+
+; Function Attrs: noinline nounwind optnone
+define i32 @main() #0 {
+entry:
+  ret i32 0
+}
+
+;CHECK:         .csect .rodata[RO]
+;CHECK-NEXT:         .align  4
+;CHECK-NEXT: .L__const.main.cnst32:
+;CHECK-NEXT:         .llong  4611686018427387954     # 0x4000000000000032
+;CHECK-NEXT:         .long   0                       # 0x0
+;CHECK-NEXT:         .space  4
+;CHECK-NEXT:         .llong  0                       # 0x0
+;CHECK-NEXT:         .long   0                       # 0x0
+;CHECK-NEXT:         .space  4
+;CHECK-NEXT:         .align  3
+;CHECK-NEXT: .L__const.main.cnst16:
+;CHECK-NEXT:         .llong  4611686018427387926     # 0x4000000000000016
+;CHECK-NEXT:         .long   0                       # 0x0
+;CHECK-NEXT:         .space  4
+;CHECK-NEXT:         .align  3
+;CHECK-NEXT: .L__const.main.cnst8:
+;CHECK-NEXT:         .long   1073741832              # 0x40000008
+;CHECK-NEXT:         .long   0                       # 0x0
+;CHECK-NEXT:         .align  3
+;CHECK-NEXT: .L__const.main.cnst4:
+;CHECK-NEXT:         .short  16392                   # 0x4008
+;CHECK-NEXT:         .byte   0                       # 0x0
+;CHECK-NEXT:         .space  1
+
+
+;CHECKOBJ:      00000000 .text:
+;CHECKOBJ-NEXT:        0: 38 60 00 00                    li 3, 0
+;CHECKOBJ-NEXT:        4: 4e 80 00 20                    blr
+;CHECKOBJ-NEXT:          ...{{[[:space:]] *}}
+;CHECKOBJ-NEXT: 00000010 .rodata:
+;CHECKOBJ-NEXT:        10: 40 00 00 00                    bdnzf   0, .+0
+;CHECKOBJ-NEXT:        14: 00 00 00 32                    <unknown>
+;CHECKOBJ-NEXT:          ...{{[[:space:]] *}}
+;CHECKOBJ-NEXT: 00000030 .L__const.main.cnst16:
+;CHECKOBJ-NEXT:       30: 40 00 00 00                    bdnzf   0, .+0
+;CHECKOBJ-NEXT:       34: 00 00 00 16                    <unknown>
+;CHECKOBJ-NEXT:          ...{{[[:space:]] *}}
+;CHECKOBJ-NEXT: 00000040 .L__const.main.cnst8:
+;CHECKOBJ-NEXT:       40: 40 00 00 08                    bdnzf   0, .+8
+;CHECKOBJ-NEXT:       44: 00 00 00 00                    <unknown>{{[[:space:]] *}}
+;CHECKOBJ-NEXT: 00000048 .L__const.main.cnst4:
+;CHECKOBJ-NEXT:       48: 40 08 00 00                    bdnzf   8, .+0
+
+
+;CHECKSYM:        Symbol {{[{][[:space:]] *}}Index: [[#Index:]]{{[[:space:]] *}}Name: .rodata
+;CHECKSYM-NEXT:     Value (RelocatableAddress): 0x10
+;CHECKSYM-NEXT:     Section: .text
+;CHECKSYM-NEXT:     Type: 0x0
+;CHECKSYM-NEXT:     StorageClass: C_HIDEXT (0x6B)
+;CHECKSYM-NEXT:     NumberOfAuxEntries: 1
+;CHECKSYM-NEXT:     CSECT Auxiliary Entry {
+;CHECKSYM-NEXT:       Index: [[#Index+1]]
+;CHECKSYM-NEXT:       SectionLen: 60
+;CHECKSYM-NEXT:       ParameterHashIndex: 0x0
+;CHECKSYM-NEXT:       TypeChkSectNum: 0x0
+;CHECKSYM-NEXT:       SymbolAlignmentLog2: 4
+;CHECKSYM-NEXT:       SymbolType: XTY_SD (0x1)
+;CHECKSYM-NEXT:       StorageMappingClass: XMC_RO (0x1)
+;CHECKSYM-NEXT:       StabInfoIndex: 0x0
+;CHECKSYM-NEXT:       StabSectNum: 0x0
+;CHECKSYM-NEXT:     }
+;CHECKSYM-NEXT:   }
+;CHECKSYM-NEXT:   Symbol {
+;CHECKSYM-NEXT:     Index: [[#Index+2]]
+;CHECKSYM-NEXT:     Name: .L__const.main.cnst32
+;CHECKSYM-NEXT:     Value (RelocatableAddress): 0x10
+;CHECKSYM-NEXT:     Section: .text
+;CHECKSYM-NEXT:     Type: 0x0
+;CHECKSYM-NEXT:     StorageClass: C_HIDEXT (0x6B)
+;CHECKSYM-NEXT:     NumberOfAuxEntries: 1
+;CHECKSYM-NEXT:     CSECT Auxiliary Entry {
+;CHECKSYM-NEXT:       Index: [[#Index+3]]
+;CHECKSYM-NEXT:       ContainingCsectSymbolIndex: [[#Index]]
+;CHECKSYM-NEXT:       ParameterHashIndex: 0x0
+;CHECKSYM-NEXT:       TypeChkSectNum: 0x0
+;CHECKSYM-NEXT:       SymbolAlignmentLog2: 0
+;CHECKSYM-NEXT:       SymbolType: XTY_LD (0x2)
+;CHECKSYM-NEXT:       StorageMappingClass: XMC_RO (0x1)
+;CHECKSYM-NEXT:       StabInfoIndex: 0x0
+;CHECKSYM-NEXT:       StabSectNum: 0x0
+;CHECKSYM-NEXT:     }
+;CHECKSYM-NEXT:   }
+;CHECKSYM-NEXT:   Symbol {
+;CHECKSYM-NEXT:     Index: [[#Index+4]]
+;CHECKSYM-NEXT:     Name: .L__const.main.cnst16
+;CHECKSYM-NEXT:     Value (RelocatableAddress): 0x30
+;CHECKSYM-NEXT:     Section: .text
+;CHECKSYM-NEXT:     Type: 0x0
+;CHECKSYM-NEXT:     StorageClass: C_HIDEXT (0x6B)
+;CHECKSYM-NEXT:     NumberOfAuxEntries: 1
+;CHECKSYM-NEXT:     CSECT Auxiliary Entry {
+;CHECKSYM-NEXT:       Index: [[#Index+5]]
+;CHECKSYM-NEXT:       ContainingCsectSymbolIndex: [[#Index]]
+;CHECKSYM-NEXT:       ParameterHashIndex: 0x0
+;CHECKSYM-NEXT:       TypeChkSectNum: 0x0
+;CHECKSYM-NEXT:       SymbolAlignmentLog2: 0
+;CHECKSYM-NEXT:       SymbolType: XTY_LD (0x2)
+;CHECKSYM-NEXT:       StorageMappingClass: XMC_RO (0x1)
+;CHECKSYM-NEXT:       StabInfoIndex: 0x0
+;CHECKSYM-NEXT:       StabSectNum: 0x0
+;CHECKSYM-NEXT:     }
+;CHECKSYM-NEXT:   }
+;CHECKSYM-NEXT:   Symbol {
+;CHECKSYM-NEXT:     Index: [[#Index+6]]
+;CHECKSYM-NEXT:     Name: .L__const.main.cnst8
+;CHECKSYM-NEXT:     Value (RelocatableAddress): 0x40
+;CHECKSYM-NEXT:     Section: .text
+;CHECKSYM-NEXT:     Type: 0x0
+;CHECKSYM-NEXT:     StorageClass: C_HIDEXT (0x6B)
+;CHECKSYM-NEXT:     NumberOfAuxEntries: 1
+;CHECKSYM-NEXT:     CSECT Auxiliary Entry {
+;CHECKSYM-NEXT:       Index: [[#Index+7]]
+;CHECKSYM-NEXT:       ContainingCsectSymbolIndex: [[#Index]]
+;CHECKSYM-NEXT:       ParameterHashIndex: 0x0
+;CHECKSYM-NEXT:       TypeChkSectNum: 0x0
+;CHECKSYM-NEXT:       SymbolAlignmentLog2: 0
+;CHECKSYM-NEXT:       SymbolType: XTY_LD (0x2)
+;CHECKSYM-NEXT:       StorageMappingClass: XMC_RO (0x1)
+;CHECKSYM-NEXT:       StabInfoIndex: 0x0
+;CHECKSYM-NEXT:       StabSectNum: 0x0
+;CHECKSYM-NEXT:     }
+;CHECKSYM-NEXT:   }
+;CHECKSYM-NEXT:   Symbol {
+;CHECKSYM-NEXT:     Index: [[#Index+8]]
+;CHECKSYM-NEXT:     Name: .L__const.main.cnst4
+;CHECKSYM-NEXT:     Value (RelocatableAddress): 0x48
+;CHECKSYM-NEXT:     Section: .text
+;CHECKSYM-NEXT:     Type: 0x0
+;CHECKSYM-NEXT:     StorageClass: C_HIDEXT (0x6B)
+;CHECKSYM-NEXT:     NumberOfAuxEntries: 1
+;CHECKSYM-NEXT:     CSECT Auxiliary Entry {
+;CHECKSYM-NEXT:       Index: [[#Index+9]]
+;CHECKSYM-NEXT:       ContainingCsectSymbolIndex: [[#Index]]
+;CHECKSYM-NEXT:       ParameterHashIndex: 0x0
+;CHECKSYM-NEXT:       TypeChkSectNum: 0x0
+;CHECKSYM-NEXT:       SymbolAlignmentLog2: 0
+;CHECKSYM-NEXT:       SymbolType: XTY_LD (0x2)
+;CHECKSYM-NEXT:       StorageMappingClass: XMC_RO (0x1)
+;CHECKSYM-NEXT:       StabInfoIndex: 0x0
+;CHECKSYM-NEXT:       StabSectNum: 0x0
+;CHECKSYM-NEXT:     }
+;CHECKSYM-NEXT:   }