[PATCH] ARM: Form paired load after stack coloring.

[Andrew Trick]

On Jun 6, 2013, at 5:00 PM, Quentin Colombet <qcolombet at apple.com> wrote:

> Hi,
> 
> ** Context **
> Recently lifetime markers have been generated by clang in the IR to be able to take advantage of the stack coloring pass.
> When these markers are set, the stack coloring pass changes, among other things, the value (setValue) attached to MachineMemoryOperand. In particular, this optimization is setting the value to 0 (<unknown>) when it cannot determine that the value does not alias with a merged alloca (StackColoring.cpp:540).
> 
> In the ARM backend, the load/store optimizer checks the presence of this value to decide whether it should form paired load or paired store.
> 
> Therefore, the stack coloring pass disables the capabilities of the ARM backend to form paired load or paired store before register allocation.
> <rdar://problem/13978317>
> 
> ** Proposed Solution **
> The attached patch removes the dependence of the ARM backend on the value (in fact, this was a false dependency, at least from what I understood).
> In particular, it does not change the stack coloring pass, as setting the value to 0 seems the right thing to do for this analysis.
> 
> ** Test Cases **
> The patch adds one test case of a paired load formed when lifetime marker are present (prior to this patch, the paired load is not formed for this test case).
> The patch also modifies test/CodeGen/ARM/2012-10-04-AAPCS-byval-align8.ll. Indeed, because of the patch, in that test, we are able to form a paired load before register allocation. Because of a bad interaction with the coalescer, the register allocator is not able to set a proper pair for the paired load. This is causing the pair loaded to be break apart after register allocation.
> Prior to this patch, the pair loaded was caught after register allocation, because both copies were coalesced. Anyway, the register allocator has not clue it may form a paired load when coalescing those.
> If the patch is accepted, I will file an enhancement request against the register allocator.
> 
> ** Action Required **
> - For Stack coloring pass expert: Confirm that the pass cannot preserve the value of the MachineMemoryOperand. I think the dependence of the ARM back on that is artificial but if we can preserve some information, why not!

I agree with Nadav that we cannot preserve the IR value. I would only add that we really want to preserve MachineMemOperands in this case, but it would require reimplementing stack coloring in IR.

> - For ARM backend expert: Confirm that the value is not required. Or put differently, what was the purpose checking if  a value was set?

AFAICT, the value is not currently required. It will be required when we decide to use AliasAnalysis in the LoadStoreOptimizer.

Essentially, if we ever want to rely on AliasAnalysis after StackColoring, we need to reimplement StackColoring at IR level. For now your patch LGTM.

-Andy

> Thanks for the review,
> 
> -Quentin
> <Mail Attachment>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> 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/20130610/e4f98c2f/attachment.html>