[PATCH] Implement ADRP CSE for global symbols

Quentin Colombet qcolombet at apple.com
Mon Apr 28 10:42:14 PDT 2014


Hi Jiangning,

> If we don't set MergedGlobal's alignment to RoundUpToPowerOfTwo(sizeof(MergedGlobal)), both syntax A and B are incorrect.

Yes, they are incorrect and that’s why I said we should go for your first approach:
adrp r0, MergedGlobal at PAGE
add  r0, r0, MergedGlobal at PAGEOFF
load [r0, #var1_offset]
load [r0, #var2_offset]
load [r0, #var3_offset]


> But if we set MergedGlobal's alignment to RoundUpToPowerOfTwo(sizeof(MergedGlobal)), both syntax A and B should be correct.

Yes, but my point is, we do not want to have this big alignment. Especially because it may make sense *only* for ARM64. Moreover, the fact that this alignment works is because both ADD and LDR have the same encoding space for the immediate. In my opinion this is a lot of assumptions that would be hard to match for all targets to make that a desirable change.

Therefore, I think we should go for your approach #1 and set a *reasonable* alignment based on the data layout.
Otherwise, you will have to demonstrate that this big alignment is desirable for all targets.

Thanks,
-Quentin

http://reviews.llvm.org/D3432






More information about the llvm-commits mailing list