[llvm] r208934 - Implement global merge optimization for global variables.

[Rafael Espíndola]

> I'm not sure if I'm understanding your point correctly. There might be
> several different functions in a module(file), and you are asking for
> concatenating symbols being used in a given function, so how do we handle
> multiple multiple functions? Can you clarify this?

Yes. The proposed profitability heuristic would be to concatenate
globals A and B if we see at least one function where A and B are
used. So for example:

* F uses A and B.
* G uses B and C.
* H uses D and E.

The proposed heuristic would decide to form the merged globals ABC and DE.

>One solution I could have
> is we use a heuristic rules to concatenating symbols in a given function as
> possible as we could, but we still need to merge all symbols all together
> into a single MergedGlobal symbol. Or you mean you want to define multiple
> MergedGlobal symbols and map them to each function? If this is the case, how
> would you handle the conflict among different functions?

Merge the groups as in the above example with F and G.

> If essentially you are proposing to remove the unused symbol of current
> module in MergedGlobal variable. I think this is a good point to me, because
> anyway it's useless on helping optimization current module.

It is a bit more than that, but yes, that would be the main benefit.

>>
>>
>> It should also be done really late. In particular, it should probably
>> not run with -flto and instead be enabled in the LTO pipeline.
>
>
> I don't understand your point here. What should be done late? Do you mean
> the GlobalMerge pass should be done late? At present, this pass happens in
> PreISel stage, so where do you want to put it?
>
> This optimization itself doesn't rely on LTO at all. But at present ARM64
> already has LTO solution using LOH, we are thinking it can be orthogonal
> with this static global merge solution. Yes, LTO is useful, but it can
> replace static solution, and a lot of end-users prefer to use static
> solution only.

Sorry, it looks like this is correct already. I misread the code, but
let me explain what I meant.

When this is run with just "clang -c -O3" the pass should be run just
before CodeGen, and that is what we get. -debug-pass=Arguments shows
it running just after -codegenprepare.

When this is run with "clang -c -O3 -flto" the pass should not be run
by clang, as the linker will be in a better spot to run it. It is true
that clang doesn't run it with -O3 -flto: -debug-pass=Arguments
doesn't list it at all.

Running llvm-lto then shows that it it runs -global-merge.

> Previously you also mentioned getGlobalMergeAlignment is out of place, so if
> you are talking about it early to to set alignment for GlobalMerge variabls,
> then could you please let me know where you think we should put it? I
> thought it was quite natural to set it at the place of generating this
> GlobalMerge symbol in global merge pass.

Oh, the out of place comment is more on the lines of "why is this
virtual"? We should be able to compute the necessary alignment in a
target independent (other than the DataLayout) way, no? The logic
could be:

* There is an explicit alignment in a global: use it.
* There is no explicit alignment, but we have a DataLayout: use the
value in the DataLayout.
* There is no explicit alignment and we don't have a DataLayout: don't
concatenate this variable.

> Sorry what does TU stand for?

Translation unit.

> The benefit for AArch64 is to reduce the ADRP (get page address of global
> symbol) and ADD instructions.
> (1) If two symbols are merged in the same data structure, they can share the
> same base address.
> (2) the offset of symbols to the page address is less than a page, offset
> can be encoded into instruction, and we will save ADD instruction, and this
> is why we want to set alignment in global merge pass.
>
> Hopefully I described the problem clearly.

Yes, thanks.


> I noticed that you already sent out the patch of supporting alias offset, so
> appreciate your quick patch, although I also noticed there is a long
> discussion around your previous commit of removing ConstantExpr for alias.
> :-) Personally I'm really hoping your alias offset support patch can go into
> trunk as soon as possible, and then recover my global merge patch.

Thanks. I will post an update version of your patch as soon as that one is in.

Cheers,
Rafael