On Oct 29, 2012, at 2:38 PM, Eli Friedman <[email protected]> wrote:
> On Mon, Oct 29, 2012 at 10:16 AM, manman ren <[email protected]> wrote: >> >> If Homogeneous Aggregate can only partially fit into VFP registers, we add >> padding to make sure >> HA will be on stack and later VFP CPRCs will be on stack as well. >> >> There is a discussion on whether we should use byval to pass things on stack: >> llvm-commits: ABI: how to let the backend know that an aggregate should be >> allocated on stack >> >> For now, the patch implemented what Eli suggested. > > + llvm::Type *PaddingTy = llvm::VectorType::get( > + llvm::Type::getFloatTy(getVMContext()), NumVFPs - PreAllocation); > > Using illegal vector types here isn't a good idea, even if it may work > for the moment. Hi Eli, Thanks for reviewing. Do you have any suggestion on what type to use as padding here? > > + it->info = ABIArgInfo::getDirect(0, 0, PaddingTy); > > getExpandWithPadding would be a bit more clear here. Will do. > > + AllocatedVFP = llvm::RoundUpToAlignment(AllocatedVFP, > + getContext().getTypeSize(Base) == 64 ? 2 : 4); > + AllocatedVFP += Members * (getContext().getTypeSize(Base) == 64 ? > 2:4); > > Use a separate variable for "getContext().getTypeSize(Base) == 64 ? > 2:4". Also, I'm not sure we can actually assume all vectors are > either 32-bit or 64-bit. Will use a separate variable. "Base" here is from isHomogeneousAggregate: // Homogeneous aggregates for AAPCS-VFP must have base types of float, // double, or 64-bit or 128-bit vectors. > > + else { > + AllocatedVFP = llvm::RoundUpToAlignment(AllocatedVFP, 2); > + AllocatedVFP += Members * 2; // Base type is double. > + } > > Please assert that the base type is double or long double. Will do. Thanks, Manman > > -Eli _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
