[llvm-commits] PATCH: Followup to r152556: Use more powerful instsimplify when actually performing the inline of a function

[Chandler Carruth]

On Mon, Mar 19, 2012 at 9:10 PM, Chris Lattner <sabre at nondot.org> wrote:

> - The instsimplify generalizations look fine to me, please commit them as
> a separate patch though.
>

Cool. Turns out almost none of them were needed. See below. =] I've
committed the one that I did need in


> - I like the patch to Inliner.cpp :-)
> - The rest of the patch looks good, one typeo: "entriy".
>
> As an effort to save some compile time, you could skip calling
> SimplifyInstOperands if none of the operands in FoldMappedInstruction were
> remapped.  If the code going into the inliner is already simplified (I
> think it should be due to instcombine running early?) then the non-remapped
> case shouldn't be simplifiable anyway.
>

So, Duncan had a great idea, which took me back to the first way I tried to
do this -- just clone the instruction, and then run the simplify routine on
the instruction. I worried about whether this would be more costly, but I
actually think it's *cheaper* here. We were already going to do the
remapping of every operand to the instruction, we just did it in a late
pass over the instructions. That isn't terribly efficient to start with, it
forces us to walk the instruction list twice rather than once. Worse, we do
the exact same remapping of each operand in both places, taking two trips
through the value remapper in the common case where we can't simplify the
instruction away.

So I changed the code to do the operand remapping eagerly (for non-PHI
nodes) and to directly simplify the cloned instructions. Much simpler now I
think. =] I've attached the new patch.

There are a couple of things I want to do as follow-ups: simplify PHINodes
to catch cases where all phi operands end up equal. This will in-turn want
to recursively apply that simplification to users of the PHINodes. This has
to happen in the lazy PHINode resolution pass, so I wanted to do it in a
follow-up patch.

Finally, this removes the Fold... method because it contained a single
function call to SimplifyInstruction by the time I was done. I'd be
interested in doing some general tidying of this code, preferably after I
stop experimenting with features:
1) condense the pile-o-arguments
2) maybe split some of the code up since the original split didn't work
3) maybe de-class-ify this since we're not really saving any argument
passing now

#2 and #3 are potentially exclusive, i wont know until i go into janitor
mode. Let me know if this would be good, and whether I should just commit
these cleanups (and the PHINode optimization) directly, or send 'em out to
the mailing list.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120321/c967726a/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: inline-instsimplify.patch
Type: application/octet-stream
Size: 8120 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120321/c967726a/attachment.obj>