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. 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
