<div dir="ltr">On Tue, Jun 25, 2013 at 12:09 PM, Bob Wilson <span dir="ltr"><<a href="mailto:bob.wilson@apple.com" target="_blank" class="cremed">bob.wilson@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: bwilson<br>
Date: Tue Jun 25 14:09:50 2013<br>
New Revision: 184870<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=184870&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=184870&view=rev</a><br>
Log:<br>
Fix SROA to avoid unnecessary scalar conversions for 1-element vectors.<br>
<br>
When a 1-element vector alloca is promoted, a store instruction can often be<br>
rewritten without converting the value to a scalar and using an insertelement<br>
instruction to stuff it into the new alloca.  This patch just adds a check<br>
to skip that conversion when it is unnecessary.  This turns out to be really<br>
important for some ARM Neon operations where <1 x i64> is used to get around<br>
the fact that i64 is not a legal type.<br></blockquote><div><br></div><div>I have a few questions about this patch, mostly to better understand what use cases you're attacking.</div><div><br></div><div>First, you mention the <1 x i64> type being important on ARM -- in what precise way is it important? Is it important in arguments and return values for ABI reasons (and to deal with the fact that there isn't an i64 GPR)? Is it important throughout the code? The reason I ask this is that it seems almost like we should teach instcombine to canonicalize single element vectors to be just scalars inside the function body, as that will simplify any number of other aspects of this problem... And if the actual generated code needs to wrap the i64 into a vector to make it legal and efficient, it feels like that should happen in codegen rather than in the middle end. None of that makes me dislike the patch here of course, it still makes perfect sense.</div>
<div><br></div><div>My second question is if this is really specific to single element vectors. Can you trigger something similar when SROA tries to split a 4 element vector alloca into two 2 element vector allocas if there happen to be some 2 element vector stores? If so, it would be good to add some testing around that.</div>
<div><br></div><div>Generally, the vector rewriting in SROA is the least well tested, so the more testing you can do to try to ensure we get this right in all inputs the better... I'm always worried about corner cases slipping through.</div>
</div></div></div>