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. -Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
