Re: [PATCH] SPARC psABI fix, callee returns struct checking.

[Eric Botcazou <ebotcazou at libertysurf dot fr>]

> Second round. I've incorporated the following recommendations:
>
> - Do not glob the logic into an pattern.
> 	= It is now emitted as RTL.
> - Machine flag name is "-mstd-struct-return"
> - The optimization is not enabled by default.
> 	= Previous compiler behaviour is preserved.

Thanks.  I leave the bulk of the review to Richard, but a few remarks:

@@ -827,6 +827,9 @@ sparc_override_options (void)
   if (!(target_flags_explicit & MASK_LONG_DOUBLE_128))
     target_flags |= MASK_LONG_DOUBLE_128;
 #endif
+
+  if (TARGET_ARCH64 && sparc_std_struct_return)
+    error ("-mstd-struct-return is not valid for 64 bit systems");
 }
  
Any particular reason for not silently accepting it with -m64?  After all, 
structure return is already standard in 64-bit mode.  Just add !TARGET_ARCH64
around the new code in sparc_struct_value_rtx.

+;; Adjust the return address conditionally. If the value of op1 is equal
+;; to all zero then adjust the return address e.g. op0 = op0 + 4.

I think you want "i.e.", not "e.g." here.

+;; Adjust the return address conditionally. If the value of op1 is equal
+;; to all zero then adjust the return address e.g. op0 = op0 + 4.
+;; This is technically *half* the check required by the 32-bit SPARCI

Typo in last word.

+;; regular rtl because the compiler kills the add thinking that the return
+;; address should always be constant.

RTL.

+(define_insn "adjust_register"
+  [(unspec:SI [(match_operand:SI 0 "register_operand" "r")
+              (match_operand:SI 1 "small_int_operand" "I")] 
UNSPEC_ADJUST_REGISTER)]
+  ""
+{
+  return "add\t%0, %1, %0\n\t";
+}
+  [(set (attr "length") (const_int 1))])

Add !TARGET_ARCH64 for extra-safety.

+         /* Calcualte the return object size */
+         tree size = TYPE_SIZE_UNIT (TREE_TYPE (fndecl

Typo.

+/* Test that GCC follows the SPARC 32-bit psABI with regards to
+   structure return checking in a callee. When -mstd-struct-return 
+   is specificed then gcc will emit code to skip the unimp insn. */ 
+
+/* Origin: Carlos O'Donell <carlos@codesourcery.com> */
+/* { dg-do run { target sparc-*-* } } */
+/* { dg-options "-mstd-struct-return" } */

This won't pass with -m64, add "dg-require-effective-target ilp32".

-- 
Eric Botcazou