Details

This patch imports external global variables references by a function, so that instcombine and globalopt can do some useful work (constant folding and static constructor evaluation) before elim-avail-extern either remove imported declarations or convert them back to externals. To simplify things this version only imports globals which have no outgoing references, so I don't have to care about promotion and importing debug info.

I've tested this on several large in-house projects and got 0.7-0.8% size improvement for each of them. I've also tried building LLVM/clang using this patch and also got some size improvement which is however much less impressive (several KB for some executables, about 56K for all of them in Release configuration). LLVM/clang built with this patch pass all tests successfully.

Why would the modulePath be empty? I think we should only have summaries during the thin link for defs in IR. Oh - answering my own question, I think - this is for regular LTO module summaries which are added to a dummy module. Please add comment.

Now you perform thinlto import and get following definition of Baz in main.o0.3.import.bc when no name mapping used:

%struct.Bar = type { i64, i64 }
%struct.Foo = type { i64, i64 }
; This is incorrect. We should have %struct.Foo in the 3rd field
; This happens because Foo and Bar are recursively isomorphic
; and Bar is seen first by IRLinker.
%struct.Baz = type { i64, i64, %struct.Bar }

The problem isn't specific at all to importing globals. You can easily rewrite this example to using functions (i.e gBaz -> getBaz) and get the same result. (it probably makes sense to divide this patch into two separate ones).

BTW the negative impact of those bitcasts isn't limited to evaluator. It also affects inliner behavior - see D38896