asavonic wrote:
I tested the patch more and added handling of duplicated symbols after IR
linking. We have the following cases:
1. Duplicated defined symbols - this is an error in AsmParser. For
`global-asm-symbols` we now use `ModFlagBehavior::AppendUnique`, so duplicates
are discarded by IR Mover. There is no diagnostic in ModuleSymbolTable, but
AsmParser will complain once such code reaches CodeGen.
2. Defined and undefined symbols linked together - not an error in AsmParser.
If there used to be an undefined symver, and the corresponding symbol is now
defined - such symver is also considered to be defined.
AppendUnique does not help here, because it treats metadata pairs with
different `BasicSymbolRef::Flags` as unique. Metadata is left with a mix of
defined and undefined symbols with the same name but different flags. This is
demonstrated in `llvm/test/Object/global-inline-asm.test`.
We cannot change how module metadata flags are merged beyond `AppendUnique`.
Therefore ModuleSymbolTable now performs de-duplication in `CollectAsmSymbols`
using metadata as an input. It goes through the list of `(symbol,flags)` pairs,
discards undefined symbols in favor of defined ones, and defines symvers in
presence of the corresponding defined symbol.
This is all very similar to what `AsmParser` and `RecordStreamer` do, but we
cannot call them here with just metadata.
3. Discarded symbols with `.lto_discard` - LTO adds this line to linked
assembly to instruct AsmParser to ignore a set of symbols. We have to change
`global-asm-symbols` in LTO to match.
4. Discarded symvers with `.lto_discard` - while LTO can discard symvers, this
syntax is not supported by AsmParser:
```
<inline asm>:1:22: unexpected token
.lto_discard bar, bar@VER
^
```
I don't see a way to write a test without dealing with this error, so
filtering of `global-asm-symver` is not done.
Updated the the patch and changed tests significantly. Generic tests are now in
`llvm/test/Object` - they do not call LTO and use llvm-link and llvm-nm.
The patch is more complicated than I originally anticipated, but I don't see
any other way. Maybe I'm missing something. Let me know what you think.
https://github.com/llvm/llvm-project/pull/174995
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
