[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
Dan Gohman
gohman at apple.com
Thu May 15 16:53:10 PDT 2008
On May 15, 2008, at 1:38 PM, Duncan Sands wrote:
>> + assert((isa<StructType>(Agg->getType()) || isa<ArrayType>(Agg-
>> >getType()) ||
>> + isa<VectorType>(Agg->getType())) &&
>
> Do you want to allow this on VectorType? Also, how about
> introducing a
> method for testing for whether a type is an aggregate type?
I don't know yet. It turns out that getelementptr works on vector types,
and that made me think having extractelement/insertelement support them
as well. It would make the IR more concise, because a single
extractevalue
could be used where otherwise an extractvalue followed by an
extractelement
would be needed. But it also means instcombine would have to do for
extractvalue/insertvalue what it does for extractelement/insertelement.
And there's no advantage for codegen.
>
>
>> -; 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 now? Before with GEP with constant indices, you
> would
> always get a constant offset from the original pointer. The value
> only
> depended on the structure of the type, not the actual value of the
> object.
> But if you are allowing going down into pointers inside structs then
> this
> is no longer true - the value no longer depends just on the type of
> %X.
> Or am I misunderstanding?
You're right. I mistook what this test was doing. I'll revert this
test change and fix the new getIndexedType code to reject this.
Thanks,
Dan
More information about the llvm-commits
mailing list