[llvm-commits] [llvm-gcc-4.2] r47077 - /llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp

[Dale Johannesen]

On Feb 15, 2008, at 11:38 AM, Chris Lattner wrote:

>
> On Feb 13, 2008, at 10:36 AM, Dale Johannesen wrote:
>
>> Author: johannes
>> Date: Wed Feb 13 12:36:09 2008
>> New Revision: 47077
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=47077&view=rev
>> Log:
>> Don't omit copying of PaddingElements; this causes
>> wrong code when structs are identical except that
>> one has padding in the same place another has a
>> real field.  Look at the right node when looking
>> for MODIFY under RET.
>
> I think this makes isPaddingElement and a bunch of code in llvm-
> types.cpp dead.

Yes, thought I'd see how this patch went first:)

> Doesn't this also cause us to regress on PR1278?

If that's what you want to call it.  Personally I think padding should  
be
deterministic whenever possible, even though the standards don't
require it; you get fewer Interesting bugs that way.

> I'm fine with temporarily saying "we'll take a code pessimization for
> improved correctness" but we need to address this somehow, perhaps by
> filing another bug.
>
> -Chris
>
>
>>
>>
>>
>> Modified:
>>   llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
>>
>> Modified: llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp?rev=47077&r1=47076&r2=47077&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> = 
>> =====================================================================
>> --- llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp (original)
>> +++ llvm-gcc-4.2/trunk/gcc/llvm-convert.cpp Wed Feb 13 12:36:09 2008
>> @@ -1246,8 +1246,6 @@
>>    const StructLayout *SL = getTargetData().getStructLayout(STy);
>>    Constant *Zero = ConstantInt::get(Type::Int32Ty, 0);
>>    for (unsigned i = 0, e = STy->getNumElements(); i != e; ++i) {
>> -      if (isPaddingElement(STy, i))
>> -        continue;
>>      Constant *Idx = ConstantInt::get(Type::Int32Ty, i);
>>      Value *Idxs[2] = { Zero, Idx };
>>      Value *DElPtr = Builder.CreateGEP(DestLoc.Ptr, Idxs, Idxs + 2,
>> "tmp");
>> @@ -1721,7 +1719,8 @@
>>    // operand is an aggregate value, create a temporary to evaluate
>> it into.
>>    MemRef DestLoc;
>>    const Type *DestTy = ConvertType(TREE_TYPE(TREE_OPERAND(exp, 0)));
>> -    if (!DestTy->isFirstClassType() && TREE_CODE(exp) !=  
>> MODIFY_EXPR)
>> +    if (!DestTy->isFirstClassType() &&
>> +        TREE_CODE(TREE_OPERAND(exp, 0)) != MODIFY_EXPR)
>>      DestLoc = CreateTempLoc(DestTy);
>>    Emit(TREE_OPERAND(exp, 0), DestLoc.Ptr ? &DestLoc : NULL);
>>  }
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits