[PATCH][DAGCombiner] insert_vector_elt: Avoid to create multiple uses of the same load.

[Nadav Rotem]

Hi Quentin, 

Thanks for working on this. I think that the problem of folding the load is secondary compared to the problem of inefficient insert sequence because of this optimization.  In your example below the code for creating V2 and V3 is inefficient because you need to build the vector twice (think about 8-wide vectors), even if “a" is not a load.  I would solve this problem by checking if the BUILD_VECTOR that we want to optimize hasOneUse(). 

Thanks,
Nadav

On Jul 24, 2013, at 5:35 PM, Quentin Colombet <qcolombet at apple.com> wrote:

> Hi,
> 
> Here is a patch that prevents DAGCombiner to combine insert_vector_elt and build_vector nodes into build_vector nodes when it creates multiple uses of the same load.
> 
> 
> ** Context **
> 
> Prior to this patch DAGCombine combines insert_vector_elt nodes with constant indexes and build_vector nodes into build_vector nodes.
> I.e.,
> insert_vector_elt (build_vector elt0, …, eltN), NewEltIdx, idx
> =>
> build_vector elt0, …, NewEltIdx, …, eltN
> 
> This has the advantage of flattening the DAG and making the vectors more obvious.
> However, if both build_vector nodes are meant to exist side by side, elt0 to eltN now have two users instead of one.
> 
> If one of this element is a load, this will prevent all folding opportunities.
> 
> 
> ** Example **
> 
> a = load
> b = load
> c = load
> V1 = insert_vector undef, a, 0 // create a new vector form a
> V2 = insert_vector V1, b, 1    // create a new vector V1 | b
> V3 = insert_vector V1, c, 1    // create a new vector V1 | c
> 
> During DAG combine, this is optimized into:
> a = load
> b = load
> c = load
> V2 = build_vector a, b    // create a new vector a | b
> V3 = build_vector a, c    // create a new vector a | c
> 
> Now the load of ‘a’ has two users. This is what it is exposed in the test case of the patch.
> 
> 
> ** Proposed Solution **
> 
> When combining an insert_vector_elt with a build_vector, we check if the vectors (the one issued from the insert_vector_elt and the one issued from the initial build_vector) are meant to exist side by side.
> If they are, we check all the sources that will be duplicated from the input vector (i.e., all but eltIdx in the argument of the input build_vector) and when one of them is a load then do not combine.
> 
> To sum up:
> - check if the resulting vector will live with the input vector.
> - if yes, check the arguments of the input vector.
>    - if one of the argument is a load used only once: do not combine as it will create an unfoldable load.
> 
> 
> Thanks for the reviews.
> 
> Cheers,
> 
> -Quentin
> <InsertVectorCombine.svndiff>
> _______________________________________________
> 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/20130724/5eb2df37/attachment.html>