On Oct 16, 2012, at 12:22 PM, manman ren <[email protected]> wrote: > > > On Oct 16, 2012, at 12:04 PM, manman ren <[email protected]> wrote: > >> >> Committed as r166040. >> Thanks Eli for reviewing. >> >> Manman >> >> On Oct 16, 2012, at 11:53 AM, Eli Friedman <[email protected]> wrote: >> >>> On Tue, Oct 16, 2012 at 11:42 AM, Manman Ren <[email protected]> wrote: >>>> >>>> On Oct 16, 2012, at 11:32 AM, Eli Friedman wrote: >>>> >>>>> On Tue, Oct 16, 2012 at 11:10 AM, manman ren <[email protected]> wrote: >>>>>> >>>>>> On Oct 15, 2012, at 5:59 PM, Eli Friedman <[email protected]> wrote: >>>>>> >>>>>>> On Mon, Oct 15, 2012 at 5:38 PM, Manman Ren <[email protected]> wrote: >>>>>>>> >>>>>>>> On Oct 15, 2012, at 5:34 PM, Eli Friedman wrote: >>>>>>>> >>>>>>>>> On Mon, Oct 15, 2012 at 5:26 PM, Manman Ren <[email protected]> wrote: >>>>>>>>>> >>>>>>>>>> On Oct 15, 2012, at 5:11 PM, Eli Friedman wrote: >>>>>>>>>> >>>>>>>>>>> On Mon, Oct 15, 2012 at 4:46 PM, manman ren <[email protected]> wrote: >>>>>>>>>>>> >>>>>>>>>>>> The initial patch was updated and separated to two patches >>>>>>>>>>>> (attached). >>>>>>>>>>>> >>>>>>>>>>>> The first patch will fix passing legal vector types as varargs: >>>>>>>>>>>> We make sure the vector is correctly aligned before casting it to >>>>>>>>>>>> the vector type. >>>>>>>>>>> >>>>>>>>>>> + uint64_t Size = CGF.getContext().getTypeSize(Ty) / 8; >>>>>>>>>>> + uint64_t TyAlign = CGF.getContext().getTypeAlign(Ty) / 8; >>>>>>>>>>> + >>>>>>>>>>> + // The ABI alignment for vectors is 8 for AAPCS and 4 for APCS. >>>>>>>>>>> + if (Ty->getAs<VectorType>() && Size >= 8) { >>>>>>>>>>> + if (getABIKind() == ARMABIInfo::AAPCS_VFP || >>>>>>>>>>> + getABIKind() == ARMABIInfo::AAPCS) >>>>>>>>>>> + TyAlign = 8; >>>>>>>>>>> + else >>>>>>>>>>> + TyAlign = 4; >>>>>>>>>>> + } >>>>>>>>>>> >>>>>>>>>>> We should be using the same rules here we do for argument passing. >>>>>>>>>>> In >>>>>>>>>>> particular, for APCS, the argument-passing type alignment is >>>>>>>>>>> unconditionally 4. (This can have effects for structs marked with >>>>>>>>>>> "__attribute__((aligned(16)))", etc.) >>>>>>>>>> Is there an interface to query the argument-passing alignment? >>>>>>>>> >>>>>>>>> Nothing cares about it other than the calling convention code, so >>>>>>>>> there isn't a general API. It's basically just "match whatever >>>>>>>>> ARMABIInfo::classifyArgumentType does". >>>>>>>> >>>>>>>> I don't see code in ARMABIInfo::classifyArgumentType that checks the >>>>>>>> argument-passing alignment. >>>>>>>> The code in the patch is trying to match what the calling convention >>>>>>>> does in the backend: >>>>>>> >>>>>>> Yes, it's a bit tricky because of the implicit contract between clang >>>>>>> and the ARM backend... you basically have to combine the frontend and >>>>>>> backend rules to figure out how the result is actually represented on >>>>>>> the stack. When you do that, it comes out to always 4 for APCS, and I >>>>>>> think it comes out to min(max(naturalAlign, 4), 8) for AAPCS. >>>>>> >>>>>> I think you are right, the patch is updated, please review again :] >>>>> >>>>> + if (Ty->getAs<VectorType>()) { >>>>> >>>>> Why restrict the check to vector types? (This check can have effects >>>>> for other types, e.g. structs containing vectors.) >>>> I want to make sure this patch only touches varargs with vector types. >>>> For other types, we can handle those in a separate patch with additional >>>> testing cases. > > Committed the patch for illegal vector types in r166043. > I will try to remove the check for vector types and add a testing case. Committed as r166052.
Thanks, Manman > > Thanks, > Manman > >>> >>> Okay. >>> >>> -Eli >> >> _______________________________________________ >> llvm-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
