The new metadata is easier to manipulate than module flags.
Details
- Reviewers
rjmccall rnk - Commits
- rG89061b222445: IR: Replace the "Linker Options" module flag with "llvm.linker.options" named…
rLLD305227: IR: Replace the "Linker Options" module flag with "llvm.linker.options" named…
rC305227: IR: Replace the "Linker Options" module flag with "llvm.linker.options" named…
rL305227: IR: Replace the "Linker Options" module flag with "llvm.linker.options" named…
Diff Detail
- Repository
- rL LLVM
Event Timeline
clang/lib/CodeGen/CodeGenModule.cpp | ||
---|---|---|
1304 ↗ | (On Diff #92992) | This is a minor behavior change, right? Now we don't deduplicate linker options for "free"? We unique the flags anyway, so we get this behavior, which seems fine: $ cat a.ll !0 = !{!"asdf"} !my_named_md = !{!0} $ cat b.ll !0 = !{!"asdf"} !my_named_md = !{!0} $ llvm-link a.ll b.ll -S -o - ; ModuleID = 'llvm-link' source_filename = "llvm-link" !my_named_md = !{!0, !0} !0 = !{!"asdf"} |
clang/lib/CodeGen/CodeGenModule.cpp | ||
---|---|---|
1304 ↗ | (On Diff #92992) | Correct. |
Clang side of this is fine.
Conceptually, I guess this is an uncommonly-enough-used extension that we can just change it without breaking too many frontends. I feel like you should probably check for the old module flag in the verifier, though; this is not something we should be silently ignoring.
After briefly attempting to rework D31352 to work without this patch, I now see why you want to do this. Working with module flags is hard and working with named md nodes is easier. Module flags still seem appropriate for integer settings that need to match during LTO, but I don't think there are many appending module flags that wouldn't be better served by named md nodes.
Done.
(I'm refreshing this patch because I want to add another named metadata kind that is lowered to object files, and this patch contained the refactoring that allowed me to do that without being forced to go through module flags. I still think this is worth doing regardless of whether we do D31352 because this representation is simpler.)
llvm/lib/Bitcode/Reader/BitcodeReader.cpp | ||
---|---|---|
2619 ↗ | (On Diff #101667) | Is there a test covering this? I see many (most?) have been updated to the new metadata. |
llvm/lib/Bitcode/Reader/BitcodeReader.cpp | ||
---|---|---|
2619 ↗ | (On Diff #101667) | Yes, see llvm/test/Bitcode/upgrade-linker-options.ll |