[PATCH] D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes

[ben via Phabricator via llvm-commits]

bd1976llvm marked an inline comment as done.
bd1976llvm added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:600
+    // sizes. Put the global into the correct existing section or create a
+    // new uniqued section.
+    if (Flags & ELF::SHF_MERGE) {
----------------
MaskRay wrote:
> We should check `Ctx.getAsmInfo()->useIntegratedAssembler()` and not use the linkage ",unique" for -no-integrated-as.
> 
> I created a feature request a few days ago https://sourceware.org/bugzilla/show_bug.cgi?id=25380
> 
@MaskRay - Thanks for the comments. This was my original design - see comments in  https://reviews.llvm.org/D68101.

However, I now think that we should land this patch in its current state and file a bug for supporting  -no-integrated-as properly. The ",unique" LLVM extension is used in other places e.g. for comdats (with no-unique-names) and for MD_associated. What do you think?

Also, I think that we should probably consider if the design of the ",unique" extension is correct. It seems a bit of a clumsy extension. Some ideas: 

1. Maybe it it would be better to have an index scheme to select between sections with the same name rather than being able to apply a unique number to any section.
2. We could have an alias scheme so that you can use a unique name for a section in assembly but specify a real name when the section is emitted into an object file.  
3. I think it might be nicer to have a new section directive e.g. .unique_section to support multiple sections with the same name rather than the current unique syntax?


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

https://reviews.llvm.org/D72194