[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

[Dale Johannesen]

On Jun 23, 2008, at 10:02 AMPDT, Chris Lattner wrote:

> 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.

The representation in the .s files is also hugely different, and much
bigger than gcc's.  I think that could be fixed in the routines that  
write it out,
but a more compact representation upstream should make that unnecessary.

The .bc and .ll files are also big.  Something like Fortran's repeated  
initializer
notation might work well:

@y = global [10000 x i32] [ i32 1, 9999 * i32 0], align 32 yadda yadda

> 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
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits