[llvm-commits] [PATCH] ARM AAPCS-VFP hard float homogeneous struct returns
Bob Wilson
bob.wilson at apple.com
Thu Oct 29 16:36:25 PDT 2009
Hi Sandeep, I've committed this. I've got some follow-on changes that
you should watch for. See comments below.
On Oct 26, 2009, at 10:21 PM, Bob Wilson wrote:
> Nice! Thanks for doing this. Some comments/questions:
>
> * Typo: DESTFIELNO -> DESTFIELDNO in the following comment:
>
> +// llvm_arm_extract_mrv_array_element - Helper function that helps
> extract
> +// an array element from multiple return value.
> +//
> +// Here, SRC is returning multiple values. DEST's DESTFIELNO field
> is an array.
> +// Extract SRCFIELDNO's ELEMENO value and store it in DEST's
> FIELDNO field's
> +// ELEMENTNO.
I fixed this for you.
>
> * Can you add an assertion at the beginning of
> llvm_arm_extract_mrv_array_element to check that Src->getType() is a
> StructType?
I did not fix this.
>
> * Likewise, at the beginning of
> llvm_arm_extract_multiple_return_value, it would be good to assert
> that all the types you cast match your expectations.
...nor this. Please submit a patch to add the assertions.
>
> * Also in llvm_arm_extract_multiple_return_value, there is a comment:
>
> + // Directly access first class values using getresult.
>
> but I don't see anything in the code involving "getresult". Just
> drop the "using getresult"?
I dropped it.
>
> * Is the following comment in
> llvm_arm_should_pass_or_return_aggregate_in_regs still relevant with
> the new containerized vector types?
>
> + // Alas, we can't use LLVM Types to figure this out because we
> need to
> + // examine unions closely. We'll have to walk the GCC TreeType.
>
> If not, please remove it.
Nevermind. I think it relevant. I was thinking that the unions in
question were something internal to arm_neon.h that might have been
removed. It is really about all unions, though.
>
> * Next time keep the whitespace changes separate. It makes it
> harder to review when you combine them.
>
> I read through the patch pretty carefully and it looks good to me,
> but I'd like to test it out. I'll try to get to that tomorrow.
I tested it. It doesn't work. See follow-on patches.....
>
> On Oct 26, 2009, at 4:37 PM, Sandeep Patel wrote:
>
>> The attached patch to llvm-gcc-4.2 fixes incorrect returns of
>> homogeneous structures. Instead of using a shadow parameter they are
>> now register allocated.
>>
>> deep
>> <deep-gcc-homogeneous-
>> returns.diff>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list