[llvm-commits] [llvm] r51157 - in /llvm/trunk: include/llvm/Constants.h include/llvm/Instruction.def include/llvm/Instructions.h include/llvm/Support/InstVisitor.h lib/AsmParser/llvmAsmParser.cpp.cvs lib/AsmParser/llvmAsmParser.h.cvs lib/AsmParser/llvmAsmParser.y lib/AsmParser/llvmAsmParser.y.cvs lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp lib/VMCore/ConstantFold.cpp lib/VMCore/ConstantFold.h lib/VMCore/Constants.cpp lib/VMCore/Instructions.cpp lib/VMCore/Verifier.cpp test/Verifier/2002-11-05-GetelementptrPointers.ll

[Chris Lattner]

On May 15, 2008, at 12:50 PM, Dan Gohman wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=51157&view=rev
> Log:
> IR support for extractvalue and insertvalue instructions. Also, begin
> moving toward making structs and arrays first-class types.

Ok.

>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- llvm/trunk/include/llvm/Constants.h (original)
> +++ llvm/trunk/include/llvm/Constants.h Thu May 15 14:50:34 2008
> @@ -575,6 +575,11 @@
>                                       Constant *Elt, Constant *Idx);
>   static Constant *getShuffleVectorTy(const Type *Ty, Constant *V1,
>                                       Constant *V2, Constant *Mask);
> +  static Constant *getExtractValueTy(const Type *Ty, Constant *Agg,
> +                                     Constant * const *Idxs,  
> unsigned NumIdxs);
> +  static Constant *getInsertValueTy(const Type *Ty, Constant *Agg,
> +                                    Constant *Val,
> +                                    Constant * const *Idxs,  
> unsigned NumIdxs);

As discussed in langref, I think this should take just "const unsigned  
*Idxs, unsigned NumIdxs" like getresult.

I understand that this means that you don't get to share code with GEP  
indexing, but it is more efficient and means that we can't have  
constantexprs as indices etc.

> // 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +//                                ExtractValueInst Class
> +// 
> = 
> = 
> = 
> ----------------------------------------------------------------------= 
> ==//
> +
> +/// ExtractValueInst - This instruction extracts a value
> +/// from an aggregate value
> +///
> +class ExtractValueInst : public Instruction {
> +  ExtractValueInst(const ExtractValueInst &EVI);
> +  void init(Value *Agg, Value* const *Idx, unsigned NumIdx);
> +  void init(Value *Agg, Value *Idx);
> +
> +  template<typename InputIterator>
> +  void init(Value *Agg, InputIterator IdxBegin, InputIterator IdxEnd,
> +            const std::string &Name,
> +            // This argument ensures that we have an iterator we can
> +            // do arithmetic on in constant time
> +            std::random_access_iterator_tag) {
> +    unsigned NumIdx = static_cast<unsigned>(std::distance(IdxBegin,  
> IdxEnd));
> +
> +    if (NumIdx > 0) {

I thought the instruction required at least one index?  Likewise in a  
couple other places.

> +++ llvm/trunk/test/Verifier/2002-11-05-GetelementptrPointers.ll Thu  
> May 15 14:50:34 2008
> @@ -1,7 +1,7 @@
> -; RUN: not llvm-as < %s |& grep {Invalid getelementptr indices}
> +; RUN: llvm-as < %s
>
> -; This testcase is invalid because we are indexing into a pointer  
> that is
> -; contained WITHIN a structure.
> +; This testcase was previously considered invalid for indexing into  
> a pointer
> +; that is contained WITHIN a structure, but this is now valid.
>
> define void @test({i32, i32*} * %X) {
> 	getelementptr {i32, i32*} * %X, i32 0, i32 1, i32 0

Why is this valid?

-Chris