[llvm-commits] [llvm] r155468 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineCalls.cpp test/Transforms/InstCombine/2012-04-23-Neon-Intrinsics.ll

[Bob Wilson]

On Apr 25, 2012, at 10:47 AM, Evan Cheng <evan.cheng at apple.com> wrote:

> 
> On Apr 25, 2012, at 9:14 AM, Bob Wilson wrote:
> 
>> 
>> On Apr 24, 2012, at 11:41 PM, Chandler Carruth <chandlerc at google.com> wrote:
>> 
>>> On Tue, Apr 24, 2012 at 11:22 PM, Chris Lattner <clattner at apple.com> wrote:
>>> 
>>> On Apr 24, 2012, at 3:10 PM, Lang Hames wrote:
>>> 
>>>> Hi Chandler,
>>>>  
>>>> I'm not sure what you mean by "the right size" here? the vmull_n_* intrinsic parameters are always half the width of its result, so they'll never be "the right size" by that measure. I'll try deferring the simplification though - that might be a win.
>>>> 
>>>> No such luck - deferring simplification of the ext instructions causes this to miss a lot of opportunities.
>>>>  
>>>> Will be OOO for the next hour or so. Hopefully I'll catch you when I get back.
>>> 
>>> Hi Lang,
>>> 
>>> I think the best thing to do is to revert this patch until we can work out a solution that works.  We really don't like instcombine code that "creates some IR, then sees if it can simplify, if not remove it", as we talked about these evening.
>>> 
>>> Hey,
>>> 
>>> Just as an update, Lang already reverted the patch.
>>> 
>>> He and I looked at a bunch of different options, and I think sense all this intrinsic is, is a multiply, he's going to look into representing it using the normal vector multiply in the IR, and pattern matching it back to the instruction in the backend.
>>> 
>>> If that doesn't work for some reason (say, for example, there is something that necessitates the *exact* lowering of this intrinsic to the instruction), we talked about how to detect opportunities for simplifications, and do a more instcombine-y transform.
>> 
>> I already replied to Lang's initial posting of the patch, but there is a problem with representing it as a normal vector multiply.  We used to do it that way and had to give up and use intrinsics because we saw important cases where the sext/zext got moved to a different basic block, which caused us to fail to select the desired instruction.
> 
> I vaguely remember this. Do you remember why the multiply isn't moved together with the sext / zext?

The change was svn 128502.  The commit message doesn't have many details.  It references rdar://8832507 and rdar://9203134.  I took a quick look at 8832507, where I commented that the zext/sext was getting moved by LICM.  Presumably only one of the operands was loop-invariant, so the multiply would remain in the loop.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120425/7ad13f42/attachment.html>