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

[Matthijs Kooijman]

Hi Dan,

> The word "Scalar" here is a little confusing. Not only can it find
> vectors (though vectors are scalars, for some uses of the word scalar),
> but it also can find aggregates, if the indices don't lead all the way
> to a leaf value. How about FindExtractedValue?
I did the opposite, I renamed it to FindInsertedValue (since it looks for an
insertvalue instruction). Agreed?

I've also moved this function to ValueTracking.cpp, since I plan to reuse it
in DeadArgElim.

> It took me a bit of puzzling to figure out what BuildSubAggregate is
> necessary. In many cases, it doesn't materially optimize the code,
> since FindScalarValue knows how to dig through chains of
> insertvalues and extractvalues, and it doesn't affect SelectionDAG-
> using backends. But I think it does help expose portions of
> struct values which are unneeded, which may be useful.
Yeah, it allows for breaking up nested structs when parts of that struct are
used as first class values. I'm currently slightly improving this part of the
code, my next commit will have a nice example in the comments.

> Here and in a few other places, watch out for going over 80 columns.
Woops, fixed now.

> It's not permitted to use auto_ptr with arrays, since it uses delete and
> not delete[]. Can you change this code to use a SmartVector instead?
Hmm, didn't know that. What exactly do you mean by SmartVector? Just
std::vector? Does that guarantee that elements are consecutive? Or perhaps
SmallVector (I think those will have consecutive elements by definition...)

> > --- llvm/trunk/test/Transforms/InstCombine/extractvalue.ll (added)
> This should be extractvalue, not extractelement.
Woops again, fixed.

Gr.

Matthijs
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20080616/2e47fef0/attachment.sig>