[llvm-commits] [PATCH] ARM AAPCS-VFP hard float homogeneous struct returns

[Bob Wilson]

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.

* Can you add an assertion at the beginning of llvm_arm_extract_mrv_array_element to check that Src->getType() is a StructType?

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

* 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"?

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

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

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