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