[PATCH] D39356: [ThinLTO] Fix missing call graph edges for calls with bitcasts.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

In https://reviews.llvm.org/D39356#918908, @mehdi_amini wrote:

> > Oh ok, this is different than what thought the issue was. There isn't a correctness problem with this - the reference edge ensures correctness. It sounds like the only correctness issue then was due to the ld64 issue. (I thought there was also an issue with getting @f promoted when @ext was imported.) There isn't really a correctness issue with having both a reference and call edge though. Ideally we would have just the call edge and no reference edge, but that goes back to the earlier suggestion you had tried (changing the call site to look through pointer casts which had other issues). So I think this is fine. Sorry for the confusion.
>
> I'm confused now: was there or not a reference edges for the internal function before this patch? If yes it seems strange that the issue would be in ld64 because the imported function would reference the renamed function, how could it match the external original f()?


Here's my current understanding:

- Due to the bitcast on the call to ext() from main(), there was no call edge and therefore no importing of ext() into main(). main() did however have a reference edge to ext(), and ext() has a call edge to the static f() in that module, so we didn't do incorrect dead stripping, etc.
- Because ext() wasn't imported (due to lack of call edge), the static f() was not promoted/renamed
- There was an issue in ld64 that confused the two f() functions, per @vsapsai:

"OK, I believe I've found a place in ld64 where we adjust fixups and associate them with other atoms based on name only, without checking linkage visibility. It is in AtomSyncer as Steven has helpfully mentioned. That issue is investigated separately."


https://reviews.llvm.org/D39356