[PATCH] Implement ADRP CSE for global symbols

Jiangning Liu liujiangning1 at gmail.com
Thu Apr 24 18:25:56 PDT 2014


Hi Quentin,

I thought I misunderstood you, but looking at your explanation below, it
seems I wasn't. See my answer below,

2014-04-25 1:06 GMT+08:00 Quentin Colombet <qcolombet at apple.com>:

> Hi Jiangning,
>
> > I think you are correct the merged size can't be simply accumulated by
> indivisual global variable size, because of natural alignment requirement
> inside the merged data structure.
>
> The size is fine, we accumulate for AllocSize, i.e., with padding and such.
> What I meant is the alignment of the whole structure does not look
> reasonable :).
>
> To be more specific, I think we should set the alignment of the merge
> structure to DL->getABIAlignment(MergedTy), or something like that.
>
> The example I had in mind was:
> @a1 = global i32 align 4
> @a2 = global i32 align 4
> @a3 = global i32 align 4
> @a4 = global i32 align 4
> =>
> struct MergeGlobal {
>  i32, i32, i32, i32
> }
> With the current approach we would get an alignment of 4*4 = 16, whereas 4
> may have been enough. Anyhow, this depends on the data layout.
>

No. I think it should be 16, otherwise this MergeGlobal maybe cross page
boundary, and the offset would not be able to fused into load/store
instruction at compile-time.

If we set alignment to 16, we would be able to guarantee at compile-time
that MergeGlobal is always within a page. The load/store instruction
requires the offset is not larger than the value holding 4096 element.
Conservatively, we can treat it as a page limitation, although in reality
it only holds for bye element.

Could you please look into again my last reply at
http://reviews.llvm.org/D3432#14 containing the summary of three different
solutions? My choice is solution 2). I drew some text charts to elaborate
the idea. For your example, I'm hoping we can at compile-time generate a
single ADRP and no ADD instruction at all.

Thanks,
-Jiangning


>
> Thanks for looking into it.
>
> -Quentin
>
> http://reviews.llvm.org/D3432
>
>
>

http://reviews.llvm.org/D3432






More information about the llvm-commits mailing list