[PATCH] Implement ADRP CSE for global symbols

[Jiangning Liu]

Hi Quentin,

> >  I introduced another command line switch -global-merge-aligned.
>
> That’s a good idea, thanks!
>
> That said, shouldn’t we set the natural alignment for external globals by default? I.e., I guess externals uses will assume that at least the natural alignment is set.

I think if I don't explicit set alignment, by default it should be natural alignment for both internal and external global variables, right? The original code for internal globals doesn't explicitly set alignment either.

Now I've got an A57/A53 big-little board, so I did some experiment and the data shows there isn't really performance impact for spec2000 int on both A57 and A53 (I believe you understand I can't post the performance data directly here due to legal issue), although I do see the following adrp instruction reductions for ARM64,

bzip2     0.28%
crafty    1.20%
vpr       0.00%
vortex    4.83%
eon       0.05%
mcf       0.00%
gzip      3.04%
perlbmk   0.02%
parser    4.49%
twolf     0.00%
gap       2.08%
gcc       1.39%

I think one of reasons might be this optimization only affects global symbol address, but it is a loop invariant essentially, and it should be always hoisted out of hot loops, so at least for spec2000 we wouldn't be able to see performance impact.

Once the pseudo instruction MOVAddr issue is solved for ARM64, there might be more adrp instruction reduction, but after this change I would still not expect performance impact for spec2000, because my measurement for AArch64 didn't show performance impact for spec2000 either.

As far as this optimization improvement itself concerned, now I think code size reduction could be the key benefit, and it wouldn't hurt any performance.

Considering this patch has been posted for almost 4 weeks and we are approaching the deadline of migrating AArch64 to ARM64, do you agree to commit this patch first? If this can happen before phasing out AArch64, it will save us a lot of more efforts to rebase, because I think the final switching from AArch64 to ARM64, and renaming ARM64 to AArch64 would be a significant change on TOT.

Anyway, I would be extremely appreciative if you can understand the current situation and agree to upstream this patch first.

Thanks,
-Jiangning

>
> Anyway, I’ll give a shot to the new patch.
>
> Thanks,
> -Quentin

http://reviews.llvm.org/D3432