[cfe-commits] [PATCH] struct{bool} -> int function argument coercion emits an invalid store on ARM
Evgeniy Stepanov
eugeni.stepanov at gmail.com
Fri Feb 10 01:35:08 PST 2012
On Fri, Feb 10, 2012 at 4:34 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Thu, Feb 9, 2012 at 12:19 AM, Evgeniy Stepanov
> <eugeni.stepanov at gmail.com> wrote:
>> Thanks for taking a look!
>> Comments inline, new patch attached.
>>
>> On Thu, Feb 9, 2012 at 9:34 AM, Eli Friedman <eli.friedman at gmail.com> 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
More information about the cfe-commits
mailing list