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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

I saw this a couple of weeks ago. Apologies that I did not pay enough attention. (I complained that GNU as lacks features. Things are changing <https://sourceware.org/ml/binutils/2020-02/msg00020.html> (I'm glad that my __patchable_function_entries complaints <https://gcc.gnu.org/ml/gcc/2020-01/msg00067.html> worked:) )

https://bugs.llvm.org/show_bug.cgi?id=43457: If we did two passes in AsmPrinter, we probably could decide `sh_entsize` in the first pass.
In the second pass, we emit the two global variables to the same section. In reality we have just one pass, so we need to decide the section when we are about to emit a global variable, without knowing whether there will be another section with a different sh_entsize.
This is the conundrum faced by the -filetype=asm backend.

For the -filetype=obj backend, it is simple: when there are conflicting sh_entsize fields => change sh_entsize to 0 (disabling merging).
In GNU ld and gold, if input sections are of different sh_entsize, no merge is performed.
lld performs limited merging (https://github.com/llvm/llvm-project/blob/master/lld/test/ELF/merge-entsize2.s) but I doubt it has much value. (I noticed that explict-section-mergeable.ll output can crash lld due to a linked-to section being a SHF_MERGE section. I'll try fixing it tomorrow.)

If we can't afford doing two passes for -filetype=asm. Another choice is to update AsmParser. When two `.section` directives with different `sh_entsize` are parsed, consider them the same section and update sh_entsize to 0.
This solves a problem and is very simple. The assembler just does some tasks that the linker will eventually do, so I'll probably not call it a hack.
However, if we want to do that, we may need to ask GNU as maintainers.

I realized that there are several assembly syntax issues we need to discuss with GNU as maintainers. We should start a thread.



================
Comment at: llvm/lib/MC/MCContext.cpp:477
+std::pair<bool, unsigned>
+MCContext::getELFUniqueIDForEntsize(StringRef SectionName, unsigned Flags,
+                                    unsigned EntrySize) {
----------------
Instead of `find()`, you can use `try_emplace`. The return value will tell you whether a new element is inserted. If yes, you can increase the unique ID.


================
Comment at: llvm/lib/MC/MCContext.cpp:482
+  return (I != ELFEntrySizeMap.end()) ? std::make_pair(true, I->second)
+                                      : std::make_pair(false, 0);
+}
----------------
```
error: conditional expression is ambiguous; 'pair<[...], typename __decay_and_strip<unsigned int>::__type>' can be converted to 'pair<[...], typename __decay_and_strip<int>::__type>' and vice versa
```

0 -> `0u`


================
Comment at: llvm/test/CodeGen/X86/explict-section-mergeable.ll:6
+; CHECK: .section	.data,"aw", at progbits,unique,1
+; CHECK-NOT: .section
+; CHECK:      uniquified:
----------------
There are a plethora of CHECK-NOT patterns which make the test difficult to read.

`FileCheck %s --implicit-check-not=.section` may be useful


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

https://reviews.llvm.org/D72194