Sink linker directive computation into IRGen #32862
Conversation
lib/IRGen/GenDecl.cpp
Outdated
There was a problem hiding this comment.
I know this isn't your bug, but I'm shocked we iterate over a StringSet here without sorting it. This cannot be correct in general.
There was a problem hiding this comment.
Also, I'm pretty sure you don't need to take the StringRefs here by reference since they're trivially copyable.
There was a problem hiding this comment.
I know this isn't your bug, but I'm shocked we iterate over a StringSet here without sorting it. This cannot be correct in general.
Ouch, good spot! Looks like we also do this in writeLdAddCFileIfNeeded. @nkcsgexi, should TBDGen return a vector of symbols instead of a set?
There was a problem hiding this comment.
Also, I'm pretty sure you don't need to take the StringRefs here by reference since they're trivially copyable.
It's a (StringRef, string length) pair, but yeah I guess & is a little overkill
There was a problem hiding this comment.
@CodaFi is deterministics your concern here or do you have other concerns?
There was a problem hiding this comment.
Yes. This code builds globals into the generated LLVM module.
There was a problem hiding this comment.
Switched to returning a vector, how does this look?
Instead of taking an out parameter, have it return the set directly. Also coalesce the two overloads into a single overload that takes a `TBDGenDescriptor`.
With an inverted pipeline, IRGen needs to be able to compute the linker directives itself, so sink it down such that it can be computed by the `IRGenDescriptor`.
A couple of clients are iterating over the result, so switch to a vector to ensure we don't accidentally introduce any non-determinism.
|
@swift-ci please test |
3 participants
With an inverted pipeline, IRGen needs to be able to compute the linker directives itself, so sink it down such that it can be computed by the
IRGenDescriptor.