[PATCH] D130449: [JITLink] Introduce WeakExternal linkage type.

[Sunho Kim via Phabricator via llvm-commits]

sunho abandoned this revision.
sunho added a comment.

Copy pasting the message I sent eariler again here to make context clear:

I've confirmed a lot of behavior through toying with link.exe.
IMAGE_WEAK_EXTERN_SEARCH_NOLIBRARY:

- Impression: does not look up strong definitions from static libraries, but if a strong symbol from a static library was loaded by other means, it will use it. Strong definition inside input object files of executable was always honored – I believe this is because MSVC load all object files of executable by default
- Symbol is not visible to the outside; usually for importing functions that are to be used internally within an object file.
- Used a lot by msvc.

IMAGE_WEAK_EXTERN_SEARCH_LIBRARY:

- This one does look up strong definitions from static archives.
- I haven’t seen it used by both msvc and clang.

IMAGE_WEAK_EXTERN_SEARCH_ALIAS:

- Basically the same as IMAGE_WEAK_EXTERN_SEARCH_NOLIBRARY, but it is visible to outside.
- Used a lot by clang, never seem to be used by msvc.

Summing up, we can just use ordinary strong and weak linkage types except for IMAGE_WEAK_EXTERN_SEARCH_LIBRARY if we are not going to support alias to external symbol. (alias to external symbol is not used by both toolchains so far) I probably got confused by it always honoring strong def from object files of executable. For "force loading" object files of executable, this shouldn't be dealt inside jitlink at all. (probably inside llvm-jitllnk tool or ORC linkinglayer?)

Anyways, we definitely don't need this patch -- some part of it can be taken to implement search_library or executable object files behaviour though.

BTW, Lookup api might still be worth it even if we are not going to implement search_library. Relocations should reference symbols of other jitdylib only through dllimport stub. It makes sense to split lookup ranges.


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

https://reviews.llvm.org/D130449