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

[Quentin Colombet]

Thanks Andy.

For some reasons, I missed your reply (until then), sorry about that.

I will wait for Evan to comment on this to see if we miss something obvious, just in case.

Thanks again for the insightful comments. I appreciate.

Cheers,

-Quentin

On Jun 10, 2013, at 6:36 PM, Andrew Trick <atrick at apple.com> wrote:

> 
> 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/20130611/9089697b/attachment.html>