[PATCH] D42127: [GlobalMerge] Don't merge dllexport globals

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 23 11:18:26 PST 2018


mstorsjo added a comment.

In https://reviews.llvm.org/D42127#985263, @compnerd wrote:

> Wouldn't it be possible to merge multiple dllexport globals?
>
>   @x = dllexport global i32 0, align 4
>   @y = dllexport global i32 0, align 4
>   @z = global i32 0, align 4
>   @a = global i32 0, align 4
>   
>
> could still merge into:
>
>   @x = dllexport global i32 0, align 4
>   @z = global i32 0, align 4
>   
>
> even though technically we should be able to merge all of that into a single global and preserve the dllexport attribute.


It could be doable, but it's rather much work - way more than this patch at least.

I've looked at what's missing - it'd require adding the dllstorage enum/field from GlobalValue into GlobalAlias (not sure if this requires extending the bitcode format), hook up GlobalAlias to the emitLinkerFlagsForGlobal function. I'd like to have the bug fixed in 6.0 as well, and this feels more and more out of scope for backporting.

IMO it'd be better to go for the approach of this patch first and backport that, and then look at maybe adding dllstorage to GlobalAlias for trunk after that, if actually doable.


https://reviews.llvm.org/D42127





More information about the llvm-commits mailing list