On Fri, Feb 10, 2012 at 4:34 AM, Eli Friedman <[email protected]> wrote: > On Thu, Feb 9, 2012 at 12:19 AM, Evgeniy Stepanov > <[email protected]> wrote: >> Thanks for taking a look! >> Comments inline, new patch attached. >> >> On Thu, Feb 9, 2012 at 9:34 AM, Eli Friedman <[email protected]> wrote: >>> Indentation generally looks strange... maybe some stray tabs? >> >> Fixed. >> >>> + llvm::Value *Casted = >>> + Builder.CreateBitCast(TempAlloca, >>> llvm::PointerType::getUnqual(ConvertTypeForMem(Ty))); >>> + llvm::LoadInst *Load = Builder.CreateLoad(Casted); >>> + Builder.CreateStore(Load, Ptr); >>> >>> Please use memcpy to copy structs, not load+store. >> >> Done. >> >>> - if (llvm::StructType *STy = >>> - dyn_cast<llvm::StructType>(ArgI.getCoerceToType())) { >>> - Ptr = Builder.CreateBitCast(Ptr, >>> llvm::PointerType::getUnqual(STy)); >>> + llvm::StructType *STy = >>> dyn_cast<llvm::StructType>(ArgI.getCoerceToType()); >>> + if (STy && STy->getNumElements() > 1) { >>> >>> For the case where STy->getNumElements() == 1, do we avoid storing a >>> struct type? If not, can you fix that? >> >> Yes, as far as I can see. We go into CreateCoercedStore, then >> EnterStructPointerForCoercedAccess. > > Okay. > > + > + Builder.CreateMemCpy(Ptr, TempAlloca, DstSize, AlignmentToUse); > > It looks like you don't actually ensure that TempAlloca is sufficiently > aligned. > > Otherwise, looks fine.
Right. Fixed and committed as r150238. Thanks again. > > -Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
