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

[Jiangning Liu]

Hi Rafael,

Rafael Espíndola <rafael.espindola at gmail.com>于2014年5月17日星期六写道：

> > I think this entire optimization has the wrong design, and I think I'd
> like
> > you to revert this patch -- unless I'm mistaken of course. Let me explain
> > why and you can decide to revert.
>
> I reverted because of its use o GEPs with non-zero offsets, but now
> that I read the patch I agree with Nick's concerns.
>

I don't think Nick is understanding this issue correctly as I mentioned in
my last reply, so I'm not sure if you are really agree with him. The issue
you mentioned below is actually completely different from Nick's point.
Refer to my answer below, anyway.


>
> I also a bit confused by what is the advantage. It looks like the
> objective is to turn two global variables into a single one and have
> an alias that provides a way to get the old name.
>
> This then lets codegen use the address difference between the two
> produce better code in a function that accesses both variables.
>
>
Yes. Your understanding is correct.


> But the code does not check if there is any function accessing both
> variables that are being merged. In fact, it can merge in a variable
> that is not used at all in the current module, right?
>

Right. It's possible.


>
> The problem with that is that it will prevent the linker (even with
> LTO, unless it gets smarter) from dead stripping this variable.


Yes. This might be the side effect.


> Is
> this uncommon enough that checking for uses coming from the same
> function was not considered profitable?
>

I'm not sure if this could be profitable, because they are global variables
and we don't really know how they are being referenced in other modules.
The heuristic of considering the uses within a function for current module
would not probably give the same benefit to other modules, so I'm thinking
the heuristic strategy of merging them all together for current module
would not make big difference from the one of further checking uses within
a function for current module.

I never say my optimization patch is commonly profitable, and this is why
they are under switches control. But for some specific cases, it would be
profitable, I believe In particular, for the case that global variables are
being heavily used in the module that are defining them.


> Also, I know your change was not responsible for it, but why is this
> ARM or AARCH64 specific?It might not be as big a win on X86, but
> putting two variables that a function uses next to each other would
> also be a win there, no?
>

X86 and ARM/AArch64 have different address model, and this is because of
the so-called CSIC and RISC differences. X86 can directly encode global
address relocation into instructions, while ARM/AArch64 can only encode it
in load/store instructions. For AArch64, this is getting worse, because
ADRP instruction can only get the page address of global variable due to
the limitation of instruction encoding size (32-bit only), and we have to
introduce another add instruction to get the 64-bit address. This is why
this optimization is very meaningful to AArch64. I think you would be able
to understand this much better if you look into the very beginning part of
my patch review thread.

Yes, putting two global variables that a function next each other would be
a win, but as I mentioned above this is also heuristic because we don't
know how global variables are being used by other modules at compilation
time.


>
> In any case, I hope to have a patch adding an explicit offset to
> GlobalAlias out this weekend or very early next week.
>

OK. Thanks for letting me know that. Hopefully my patch still can reuse
your new GlobalAlias solution. Looking at the bug tracker you pointed to
me, it seems to be a very old problem raised by Christ long time ago. I'm
not sure if you can really make it happen next week. I would be extremely
appreciative if you do.

Thanks,
-Jiangning


> Thanks,
> Rafael
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <javascript:;>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140517/2ba33d8b/attachment.html>