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

Jiangning Liu liujiangning1 at gmail.com
Mon Jun 2 23:54:14 PDT 2014


Hi Rafael,

2014-06-03 11:48 GMT+08:00 Rafael EspĂ­ndola <rafael.espindola at gmail.com>:

> > Thanks. I will post an update version of your patch as soon as that one
> is in.
>
> And that took a long time, sorry about that :-(
>
> Attached is my attempt to forward port the patch. I didn't change the
> profitability check or the use of getGlobalMergeAlignment.
>

Appreciate your great effort on modifying global alias implementation and
supporting global symbol offset for global alias! Also thanks a lot for
your merging work for my patch.


>
> One issue is that the ARM64 -> AArch64 transition happened and it
> seems that for one test we are not produce as good case as before. At
> the IR level the constant is merged, be we still compute the address
> twice, not sure why,  sorry.
>
> The AArch64 in my old patch refers to the old AArch64, and we don't have
this problem for old AArch64. This is a known issue for this new
AArch64(Migrated from ARM64).

Previously Quentin kindly helped to investigated how to solve the ARM64
back-end issue, but so far it seems we it is still not sure if there is
anything wrong with Quentin's patch, so this is why I want to commit my
patch first and then I will try to pick up Quentin's patch to triage the
new AArch64 back-end solution.

Now I removed it those failure tests and added FIXME in the test case.


> Would you mind taking a look and posting this again? Do CC myself and
> Nick. If I remember correctly the only remaining concern was why
> getGlobalMergeAlignment was needed and the command line options.  A
> better profitability check would be nice, but can be just a FIXME for
> now.
>

I think there are two issues you mentioned
(1) We probably should change the global merge algorithm by considering the
global symbols function by function, which could finally benefit linker to
remove dead global symbols. I think I agree with that, but I want to leave
it for further improvement in future, so I added a FIXME in my new patch to
explain the issue.
(2) We should avoid adding target dependent interfaces in middle-end, so it
would be better we don't add getGlobalMergeAlignment. I consider it again,
and think we still need this, and it is orthogonal with (1). For AArch64,
we need this because we want to make sure the merged global variable
doesn't cross page boundary, and this could finally remove one more
"addition" instruction finally.

Thanks,
-Jiangning


> Thanks,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140603/df648679/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: adrp_llvm_5.patch
Type: text/x-patch
Size: 25011 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140603/df648679/attachment.bin>


More information about the llvm-commits mailing list