[llvm-commits] [llvm] r51806 - in /llvm/trunk: docs/LangRef.html include/llvm/Constants.h include/llvm/DerivedTypes.h include/llvm/Instructions.h lib/AsmParser/llvmAsmParser.cpp.cvs lib/AsmParser/llvmAsmParser.h.cvs lib/AsmParser/llvmAsmParser.y lib/AsmParser/llvmAsmParser.y.cvs lib/Bitcode/Reader/BitcodeReader.cpp lib/Bitcode/Writer/BitcodeWriter.cpp lib/VMCore/Constants.cpp lib/VMCore/Instructions.cpp lib/VMCore/Type.cpp test/Assembler/insertextractvalue.ll

[Chris Lattner]

On Jun 23, 2008, at 9:48 AM, Dan Gohman wrote:
>> Are insertvalue/extractvalue valid as constant expressions?  If so,
>> langref should be updated and it would be good to see that they  
>> encode
>> into the bc file correctly.  If we can *guarantee* that they are
>> always folded away by the constant folding, then I'm fine with saying
>> that they are not allowed as constant exprs.
>
> Currently we should be folding all such constant expressions.
>
> One case where we might not want to is a huge zeroinitializer with a
> few non-zero values inserted into it. It's a real situation, though I
> don't know if it's enough of a concern to justify here.

That is a really interesting case actually.  It is bad that:

int x[10000];

and:

int x[10000] = { 1 };

take hugely different amounts of memory to represent.  However, we  
already have this problem, and it could be solved in other ways (e.g.  
have some new 'SparseConstantArray' or something).  I tend to think  
that eliminating the constantexprs would be a net win, because we  
don't need to handle them everywhere (e.g. in the asmprinter logic for  
emitting global var initializers etc).

What do you think?

>>> @@ -1518,9 +1519,11 @@
>>> /// element value from an aggregate value.
>>> ///
>>> class ExtractValueInst : public Instruction {
>>> +  SmallVector<unsigned, 4> Indices;
>>
>> The indices of an instruction are fixed when the instruction is
>> created.  Would it be reasonable to just make this be a new
>> unsigned[]'d array instead of a smallvector?
>
> SmallVector is a convenient way to co-allocate the indices with
> the instruction in the common cases. We can certainly look at
> doing custom allocation strategies to allocate the indices after
> the end of the ExtractValueInst though.

True, but in practice, these (insert/extractvalue insts) will be rare  
(so it doesn't really matter one way or the other) and will almost  
always have one index.  How about SmallVector<1>?

Thanks Dan!

-Chris