[llvm-commits] [llvm] r52217 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/Transforms/InstCombine/extractvalue.ll

[Chris Lattner]

On Jun 11, 2008, at 7:05 AM, Matthijs Kooijman wrote:

> Author: matthijs
> Date: Wed Jun 11 09:05:05 2008
> New Revision: 52217
>
> URL: http://llvm.org/viewvc/llvm-project?rev=52217&view=rev
> Log:
> Teach instruction combining about the extractvalue. It can  
> succesfully fold
> useless insert-extract chains, similar to how it folds them for  
> vectors.

Hi Matthijs,

Thank you for working on this, but I think this implementation is  
major overkill :).  The issue with vectors is that it is extremely  
inefficient for vectors to make transformations like:

from:

a = { x, y, z}  // three inserts.
b = shuffle a, v1
c = shuffle b, v2
output = extract c, 1

to:

a = { x, y, z}  // three inserts.
b = shuffle a, v1
c = shuffle b, v2
output = extract b, 3

(assuming c and b have multiple other uses).  Because of this, the  
vector code has to try hard to look through all these operations and  
only transform the code if it ends up able to change an extract into  
some input scalar that was inserted.

First-class aggregates don't have this problem.  Because of this, I  
think this code can be replaced with a very simple implementation that  
basically does:

visitExtractValueInst(I) {
   op = I.getOperand(0);
   if (isa<ConstantAggZero>(op))
     use new zero;
   else if (insertvalueinst) {
     if (index is the thing inserted)
       use inserted value
     else
       use new extract value inst of aggregate input
...
}

The thing that makes this really simple is that it is perfectly okay  
to transform:

A = insertvalue B, C, 14
T = extractvalue A, 13

into:

A = insertvalue B, C, 14
T = extractvalue B, 13

and rely on the iterative nature of instcombine to simplify the new  
extractvalue away.

Does this make sense?  I think it will result in a lot less code and a  
faster implementation in common cases.

-Chris