<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div><div><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div><div></div><br><div><div>On Jul 25, 2013, at 11:15 AM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div apple-content-edited="true"><div style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><span style="text-align: -webkit-auto;">Hi Nadav,</span></div><div style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><span style="text-align: -webkit-auto;"><br></span></div><div style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><span style="text-align: -webkit-auto;">Thanks for the feedback.</span></div><div style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><span style="text-align: -webkit-auto;"><br></span></div><div style="orphans: 2; text-align: -webkit-auto; text-indent: 0px; widows: 2; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><span style="text-align: -webkit-auto;">Well, I did not want to go in that direction because I think that the instruction selection of all LLVM targets knows how to deal efficiently with build_vector nodes.</span></div><div style="orphans: 2; text-align: -webkit-auto; text-indent: 0px; widows: 2; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><span style="text-align: -webkit-auto;">Moreover, in some cases, the BUILD_VECTOR we want to optimize has several uses but the final vector has not.</span></div><div style="orphans: 2; text-align: -webkit-auto; text-indent: 0px; widows: 2; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><span style="text-align: -webkit-auto;">E.g.,</span></div><div style="orphans: 2; text-align: -webkit-auto; text-indent: 0px; widows: 2; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><span style="text-align: -webkit-auto;">V1 = insert_vector undef, cst, 0 // create a new vector from a constant.</span><br><span style="text-align: -webkit-auto;">V2 = insert_vector V1, b, 1 // create a new vector V1 | b</span><br><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>V3 = insert_vector V1, c, 1 // create a new vector V1 | c</div><div><br></div><div>With current behavior:</div><div><span style="text-align: -webkit-auto;">V2 = build_vector cst, b // create a new vector cst | b</span><br><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">V3 = build_vector cst, c // create a new vector cst | c</div><div><br></div></div></div><div>With proposed behavior:</div></div></div></div><div style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><span style="text-align: -webkit-auto;">V1 = insert_vector undef, cst, 0 // create a new vector from a constant.</span><br><span style="text-align: -webkit-auto;">V2 = build_vector V1, b // create a new vector V1 | b</span><br><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">V3 = build_vector V1, c // create a new vector V1 | c</div><div><br></div><div>I do not know if the code resulting from what you proposed will cause trouble but it is more likely to impact all the target than what I have proposed.</div><div><br></div><div>An alternative approach could be to do something between what we both proposed, in other words, checking if the build_vector we optimize has several uses and if yes, checking if the uses that we are about to duplicate are not constant.</div><div><span style="text-align: -webkit-auto;">However, I am not sure it is a good idea to allow constant duplication and prevent uses duplication generally.</span></div><div>Again, I think, but I may be wrong, most back-ends are optimized to deal with build_vector properly. The load case I addressed in that patch is something, I believe, that affects all targets.</div><div><br></div><div>What do you think?</div><div><br></div><div>Cheers,</div><div><br></div></div></div><div style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><span style="text-align: -webkit-auto;">-Quentin</span></div></div><br><div><div>On Jul 24, 2013, at 10:16 PM, Nadav Rotem <<a href="mailto:nrotem@apple.com">nrotem@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Quentin, <div><br></div><div>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(). </div><div><br></div><div>Thanks,</div><div>Nadav</div><div><br><div><div>On Jul 24, 2013, at 5:35 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi,<div><br></div><div>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.</div><div><br></div><div><br></div><div>** Context **</div><div><br></div><div>Prior to this patch DAGCombine combines insert_vector_elt nodes with constant indexes and build_vector nodes into build_vector nodes.</div><div>I.e.,</div><div>insert_vector_elt (build_vector elt0, …, eltN), NewEltIdx, idx</div><div>=></div><div>build_vector elt0, …, NewEltIdx, …, eltN</div><div><br></div><div>This has the advantage of flattening the DAG and making the vectors more obvious.</div><div>However, if both build_vector nodes are meant to exist side by side, elt0 to eltN now have two users instead of one.</div><div><br></div><div>If one of this element is a load, this will prevent all folding opportunities.</div><div><br></div><div><br></div><div>** Example **</div><div><br></div><div><div>a = load</div><div>b = load</div><div>c = load</div><div>V1 = insert_vector undef, a, 0 // create a new vector form a</div><div>V2 = insert_vector V1, b, 1 // create a new vector V1 | b</div><div>V3 = insert_vector V1, c, 1 // create a new vector V1 | c</div></div><div><br></div><div><div>During DAG combine, this is optimized into:</div><div>a = load</div><div>b = load</div><div>c = load</div><div>V2 = build_vector a, b // create a new vector a | b</div><div>V3 = build_vector a, c // create a new vector a | c</div><div><br></div><div>Now the load of ‘a’ has two users. This is what it is exposed in the test case of the patch.</div><div><br></div><div><br></div><div>** Proposed Solution **</div><div><br></div><div>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.</div><div>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.</div><div><br></div><div>To sum up:</div><div>- check if the resulting vector will live with the input vector.</div><div>- if yes, check the arguments of the input vector.</div><div> - if one of the argument is a load used only once: do not combine as it will create an unfoldable load.</div><div><br></div><div><br></div><div>Thanks for the reviews.</div><div><br></div><div>Cheers,</div><div><br></div><div apple-content-edited="true"><div style="font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">-Quentin</div></div></div><span><InsertVectorCombine.svndiff></span><div></div>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div></div></div></blockquote></div><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></div></blockquote></div></div></div></blockquote></div><br></div></body></html>