[llvm-commits] [llvm] r51461 - in /llvm/trunk: include/llvm/Instructions.h lib/VMCore/Constants.cpp lib/VMCore/Instructions.cpp
Dan Gohman
gohman at apple.com
Thu May 29 15:16:26 PDT 2008
On May 29, 2008, at 8:35 AM, Matthijs Kooijman wrote:
> Hi Dan,
>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- llvm/trunk/lib/VMCore/Instructions.cpp (original)
>> +++ llvm/trunk/lib/VMCore/Instructions.cpp Thu May 22 19:36:11 2008
>> @@ -1333,9 +1333,69 @@
> [...]
>> +void InsertValueInst::init(Value *Agg, Value *Val, Value* const
>> *Idx, unsigned NumIdx) {
>> + assert(NumOperands == 1+NumIdx && "NumOperands not initialized?");
>> + Use *OL = OperandList;
>> + OL[0].init(Agg, this);
>> + OL[1].init(Val, this);
>> +
>> + for (unsigned i = 0; i != NumIdx; ++i)
>> + OL[i+2].init(Idx[i], this);
>> +}
> From the looks of this, that assert should say 2+NumIdx, the 1 seems
> copied
> from ExtractValueInst. Or am I missing something here?
You're right. Good catch.
>
>
> Also, in an earlier revision:
>> New Revision: 51157
>
> In Instructions.h:
>> + template<typename InputIterator>
>> + static InsertValueInst *Create(Value *Agg, Value *Val,
>> InputIterator IdxBegin,
>> + InputIterator IdxEnd,
>> + const std::string &Name = "",
>> + Instruction *InsertBefore = 0) {
>> + typename std::iterator_traits<InputIterator>::difference_type
>> Values =
>> + 1 + std::distance(IdxBegin, IdxEnd);
>> + return new(Values)
>> + InsertValueInst(Agg, Val, IdxBegin, IdxEnd, Values, Name,
>> InsertBefore);
>> + }
>> + template<typename InputIterator>
>> + static InsertValueInst *Create(Value *Agg, Value *Val,
>> + InputIterator IdxBegin,
>> InputIterator IdxEnd,
>> + const std::string &Name,
>> + BasicBlock *InsertAtEnd) {
>> + typename std::iterator_traits<InputIterator>::difference_type
>> Values =
>> + 1 + std::distance(IdxBegin, IdxEnd);
>> + return new(Values)
>> + InsertValueInst(Agg, Val, IdxBegin, IdxEnd, Values, Name,
>> InsertAtEnd);
>> + }
> Shouldn't this be 2 + as well on both constructors?
Yep.
>
>
>> + /// Constructors - These two constructors are convenience
>> methods because one
>> + /// and two index insertvalue instructions are so common.
>> + InsertValueInst(Value *Agg, Value *Val,
>> + Value *Idx, const std::string &Name = "",
>> + Instruction *InsertBefore = 0);
>> + InsertValueInst(Value *Agg, Value *Val, Value *Idx,
>> + const std::string &Name, BasicBlock *InsertAtEnd);
>
> It seems these two were declared but not implemented, I will probably
> implement these now or tomorrow, unless you do it first :-)
Thanks, however I'm actually in the middle of making some significant
changes to this code. Following on review feedback, I'm going to change
it to use compile-time constant indicies (uint64_t) instead of
Constant*'s, and make some other changes that go along with that.
That change means ExtractValue and InsertValue no longer need
variable-length Use lists, and I'm still working out how things
ought to look under Gabor's new framework (it *is* saving us memory
at least, right? :-})
Dan
More information about the llvm-commits
mailing list