https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84762

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84762

[Umesh Kalappa]

[-- Attachment #1: Type: text/plain, Size: 148 bytes --]

Hi All,

Please find the attached patch for the subjected issue .

Do please let me know your thoughts and comments on the same .

Thank you
~Umesh

[-- Attachment #2: pr84762.patch --]
[-- Type: application/octet-stream, Size: 5980 bytes --]

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index ee5f183..d1c0edb 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,14 @@
+2018-12-06  Lokesh Janghel  <lokeshjanghel91@gmail.com>
+
+        PR target/84762
+        * config/rs6000/rs6000.c (rs6000_return_in_msb): Retrun in svr4 for
+	 small struct value.
+	 (rs6000_option_override_internal): Modify the condition for aix or
+	 svr4. 
+	* config/rs6000/rs6000.opt : Modify the -msvr4-struct-return option.
+	* config/rs6000/rs6000-opts.h : Add enum for svr4 option (Big endian 
+	and Little endian).
+
 2018-11-21  Uros Bizjak  <ubizjak@gmail.com>
 
 	Revert the revert:
diff --git a/gcc/config/rs6000/rs6000-opts.h b/gcc/config/rs6000/rs6000-opts.h
index 1212d11..c405a37 100644
--- a/gcc/config/rs6000/rs6000-opts.h
+++ b/gcc/config/rs6000/rs6000-opts.h
@@ -130,6 +130,14 @@ enum rs6000_cmodel {
   CMODEL_LARGE
 };
 
+/* Return small structs in register,
+   gnu: LSB-aligned,
+   standard: MSB-aligned    */
+enum rs6000_svr4_struct_return {
+  SVR4_STRUCT_RETURN_GNU=1,
+  SVR4_STRUCT_RETURN_STD
+};
+
 /* Describe which vector unit to use for a given machine mode.  The
    VECTOR_MEM_* and VECTOR_UNIT_* macros assume that Altivec, VSX, and
    P8_VECTOR are contiguous.  */
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 2765263..4751b61 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4632,7 +4632,8 @@ rs6000_option_override_internal (bool global_init_p)
       /* Set aix_struct_return last, after the ABI is determined.
 	 If -maix-struct-return or -msvr4-struct-return was explicitly
 	 used, don't override with the ABI default.  */
-      if (!global_options_set.x_aix_struct_return)
+      if (!global_options_set.x_aix_struct_return
+	   && !rs6000_current_svr4_struct_return)
 	aix_struct_return = (DEFAULT_ABI != ABI_V4 || DRAFT_V4_STRUCT_RET);
 
 #if 0
@@ -10616,8 +10617,10 @@ rs6000_return_in_memory (const_tree type, const_tree fntype ATTRIBUTE_UNUSED)
 static bool
 rs6000_return_in_msb (const_tree valtype)
 {
-  return (DEFAULT_ABI == ABI_ELFv2
-	  && BYTES_BIG_ENDIAN
+  return ((DEFAULT_ABI == ABI_ELFv2
+          || (DEFAULT_ABI == ABI_V4
+          && rs6000_current_svr4_struct_return == SVR4_STRUCT_RETURN_STD))
+ 	  && BYTES_BIG_ENDIAN
 	  && AGGREGATE_TYPE_P (valtype)
 	  && (rs6000_function_arg_padding (TYPE_MODE (valtype), valtype)
 	      == PAD_UPWARD));
diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
index 7042859..a49fb56 100644
--- a/gcc/config/rs6000/rs6000.opt
+++ b/gcc/config/rs6000/rs6000.opt
@@ -92,6 +92,10 @@ unsigned char rs6000_alignment_flags
 TargetVariable
 enum rs6000_cmodel rs6000_current_cmodel = CMODEL_SMALL
 
+;; Retrun small struct in register
+TargetVariable
+enum rs6000_svr4_struct_return rs6000_current_svr4_struct_return = SVR4_STRUCT_RETURN_GNU
+
 ;; What type of reciprocal estimation instructions to generate
 TargetVariable
 unsigned int rs6000_recip_control
@@ -258,9 +262,18 @@ maix-struct-return
 Target Report RejectNegative Var(aix_struct_return) Save
 Return all structures in memory (AIX default).
 
-msvr4-struct-return
-Target Report RejectNegative Var(aix_struct_return,0) Save
-Return small structures in registers (SVR4 default).
+msvr4-struct-return=
+Target RejectNegative Joined Enum(rs6000_svr4_struct_return)  Var(rs6000_current_svr4_struct_return)
+-msvr4-struct-return=[standard,gnu] Return small structures in registers (SVR4 default).
+
+Enum
+Name(rs6000_svr4_struct_return) Type(enum rs6000_svr4_struct_return)
+
+EnumValue
+Enum(rs6000_svr4_struct_return) String(standard) Value(SVR4_STRUCT_RETURN_STD) 
+
+EnumValue
+Enum(rs6000_svr4_struct_return) String(gnu) Value(SVR4_STRUCT_RETURN_GNU) 
 
 mxl-compat
 Target Report Var(TARGET_XL_COMPAT) Save
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index afd928e..8f36fbd 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2018-12-06  Lokesh Janghel  <lokeshjanghel91@gmail.com>
+
+        PR target/84762
+        * gcc.target/pr84762-1.c: New testcase.
+        * gcc.target/pr84762-2.c: New testcase.
+        * gcc.target/pr84762-3.c: New testcase.
+
 2018-11-21  Jakub Jelinek  <jakub@redhat.com>
 
 	PR c++/88122
diff --git a/gcc/testsuite/gcc.target/powerpc/pr84762-1.c b/gcc/testsuite/gcc.target/powerpc/pr84762-1.c
new file mode 100644
index 0000000..cb417d1
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr84762-1.c
@@ -0,0 +1,9 @@
+/* { dg-do run { target powerpc*-*-* rs6000-*-* } } */
+struct smallstruct { char a; char b; char c; };
+
+struct smallstruct f(void)
+{
+  struct smallstruct s = { 0x11, 0x22, 0x33 };
+
+  return s;
+}
diff --git a/gcc/testsuite/gcc.target/powerpc/pr84762-2.c b/gcc/testsuite/gcc.target/powerpc/pr84762-2.c
new file mode 100644
index 0000000..44305f2
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr84762-2.c
@@ -0,0 +1,12 @@
+/* { dg-do compile { target powerpc*-*-* rs6000-*-* } } */
+/* { dg-options "-O2 -msvr4-struct-return=gnu" } */
+/* { dg-final { scan-assembler-times "lis \[\n\]*,0x\[\n\]*" 1 } } */
+/* { dg-final { scan-assembler-times "ori \[\n\]*,\[\n\]*,0x\[\n\]*" 1 } } */
+struct smallstruct { char a; char b; char c; };
+
+struct smallstruct f(void)
+{
+  struct smallstruct s = { 0x11, 0x22, 0x33 };
+
+  return s;
+}
diff --git a/gcc/testsuite/gcc.target/powerpc/pr84762-3.c b/gcc/testsuite/gcc.target/powerpc/pr84762-3.c
new file mode 100644
index 0000000..f01dc83
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr84762-3.c
@@ -0,0 +1,12 @@
+/* { dg-do compile { target powerpc*-*-* rs6000-*-* } } */
+/* { dg-options "-O2 -msvr4-struct-return=standard" } */
+/* { dg-final { scan-assembler-times "lis \[\n\]*,0x\[\n\]*" 1 } } */
+/* { dg-final { scan-assembler-times "ori \[\n\]*,\[\n\]*,0x\[\n\]*" 1 } } */
+struct smallstruct { char a; char b; char c; };
+
+struct smallstruct f(void)
+{
+  struct smallstruct s = { 0x11, 0x22, 0x33 };
+
+  return s;
+}

Re: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84762

[Jakub Jelinek]

On Tue, Dec 11, 2018 at 05:30:48PM +0530, Umesh Kalappa wrote:
> Hi All,
> 
> Please find the attached patch for the subjected issue .
> 
> Do please let me know your thoughts and comments on the same .

Not a patch review (will defer that to rs6000 maintainers), but
some comments on gcc-patches patch submissions.

The subject should ideally start with [PATCH] or similar,
then have some short summary of what the patch is about and if
it fixes some PR, just PR something/12345 reference,
the subjects you are posting like:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84762
don't say anything relevant except for the PR 84762 number,
so anyone reading gcc-patches needs to open that bug in order to even find
out if it is something for him or somebody else.

If you are sending a patch for an area that has some maintainer(s),
usually you should either mention those maintainers in To: (and CC:
gcc-patches) or To: gcc-patches, CC: the maintainers, to draw their
attention.  See MAINTAINERS file in GCC tree.

The mail body should start with a short explanation of what the problem is
and how are you solving it, again, so that people don't have to jump to
bugzilla to find out (of course, short is enough, no need to duplicate
dozens of comments from the PR), should include information on what
target(s) it has been bootstrapped/regtested.  And, it is always better if
it is the patch author that posts it, or is at least CCed so that he can
answer review questions.

Thanks.

	Jakub

Re: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84762

[Jonathan Wakely]

On 11/12/18 13:28 +0100, Jakub Jelinek wrote:
>On Tue, Dec 11, 2018 at 05:30:48PM +0530, Umesh Kalappa wrote:
>> Hi All,
>>
>> Please find the attached patch for the subjected issue .
>>
>> Do please let me know your thoughts and comments on the same .
>
>Not a patch review (will defer that to rs6000 maintainers), but
>some comments on gcc-patches patch submissions.
>
>The subject should ideally start with [PATCH] or similar,
>then have some short summary of what the patch is about and if
>it fixes some PR, just PR something/12345 reference,
>the subjects you are posting like:
>https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84762
>don't say anything relevant except for the PR 84762 number,
>so anyone reading gcc-patches needs to open that bug in order to even find
>out if it is something for him or somebody else.
>
>If you are sending a patch for an area that has some maintainer(s),
>usually you should either mention those maintainers in To: (and CC:
>gcc-patches) or To: gcc-patches, CC: the maintainers, to draw their
>attention.  See MAINTAINERS file in GCC tree.
>
>The mail body should start with a short explanation of what the problem is
>and how are you solving it, again, so that people don't have to jump to
>bugzilla to find out (of course, short is enough, no need to duplicate
>dozens of comments from the PR), should include information on what
>target(s) it has been bootstrapped/regtested.  And, it is always better if
>it is the patch author that posts it, or is at least CCed so that he can
>answer review questions.

Also, as noted at https://gcc.gnu.org/contribute.html#patches the
ChangeLog entry should be in plain text, not part of the patch.

"The ChangeLog entries should be plaintext rather than part of the
patch since the top of the ChangeLog changes rapidly and a patch to
the ChangeLog would probably no longer apply by the time your patch is
reviewed."

Some people add it as a second attachment, e.g.
https://gcc.gnu.org/ml/gcc-patches/2018-11/msg02524.html
or just inline in the email body, e.g.
https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00622.html
Either is better than including the ChangeLog entry as part of the
patch itself.

Following the https://gcc.gnu.org/contribute.html#patches policies
will make it much more likely that patches will be seen by the right
people and reviewed.

Re: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84762

[Segher Boessenkool]

Hi Umesh,

On Tue, Dec 11, 2018 at 05:30:48PM +0530, Umesh Kalappa wrote:
> Please find the attached patch for the subjected issue .
> 
> Do please let me know your thoughts and comments on the same .

First of all: do you have a copyright assignment with the FSF?

Second: please don't send application/octet-stream attachments.


diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index ee5f183..d1c0edb 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,14 @@
+2018-12-06  Lokesh Janghel  <lokeshjanghel91@gmail.com>
+
+        PR target/84762
+        * config/rs6000/rs6000.c (rs6000_return_in_msb): Retrun in svr4 for
+	 small struct value.
+	 (rs6000_option_override_internal): Modify the condition for aix or
+	 svr4. 
+	* config/rs6000/rs6000.opt : Modify the -msvr4-struct-return option.
+	* config/rs6000/rs6000-opts.h : Add enum for svr4 option (Big endian 
+	and Little endian).

The changelog should not be part of the patch, but written before it.
Not as diff, just as the text it is.

Indent is one tab.  Not a tab and a space, not nine spaces.

There shouldn't be trailing spaces.

There should not be a space before a colon.

"Modify XYZ." means that you should have "(XYZ): Modify." instead; but
you probably can say more than just "Modify", too.  Like, _what_ have you
changed about it :-)

s/retrun/return/

The changelog should mention everything you change.  I haven't checked
if it does here, but all the testcases are missing (those have their own
changelog, in gcc/testsuite/ChangeLog).

+/* Return small structs in register,
+   gnu: LSB-aligned,
+   standard: MSB-aligned    */

This should end with dot space space */

+enum rs6000_svr4_struct_return {
+  SVR4_STRUCT_RETURN_GNU=1,
+  SVR4_STRUCT_RETURN_STD
+};

I think a simple boolean would be easier?

diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 2765263..4751b61 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -4632,7 +4632,8 @@ rs6000_option_override_internal (bool global_init_p)
       /* Set aix_struct_return last, after the ABI is determined.
 	 If -maix-struct-return or -msvr4-struct-return was explicitly
 	 used, don't override with the ABI default.  */
-      if (!global_options_set.x_aix_struct_return)
+      if (!global_options_set.x_aix_struct_return
+	   && !rs6000_current_svr4_struct_return)
 	aix_struct_return = (DEFAULT_ABI != ABI_V4 || DRAFT_V4_STRUCT_RET);

Why this change?

 static bool
 rs6000_return_in_msb (const_tree valtype)
 {
-  return (DEFAULT_ABI == ABI_ELFv2
-	  && BYTES_BIG_ENDIAN
+  return ((DEFAULT_ABI == ABI_ELFv2
+          || (DEFAULT_ABI == ABI_V4
+          && rs6000_current_svr4_struct_return == SVR4_STRUCT_RETURN_STD))
+ 	  && BYTES_BIG_ENDIAN
 	  && AGGREGATE_TYPE_P (valtype)
 	  && (rs6000_function_arg_padding (TYPE_MODE (valtype), valtype)
 	      == PAD_UPWARD));

Indents are with tabs, not eight spaces.  There never should be tabs
after spaces though.

Please write this as

  if (DEFAULT_ABI == ABI_ELFv2
      && BYTES_BIG_ENDIAN
      && AGGREGATE_TYPE_P (valtype)
      && (rs6000_function_arg_padding (TYPE_MODE (valtype), valtype)
	  == PAD_UPWARD))
    return true;

  if (DEFAULT_ABI == ABI_V4
      && rs6000_current_svr4_struct_return == SVR4_STRUCT_RETURN_STD
      && BYTES_BIG_ENDIAN
      && AGGREGATE_TYPE_P (valtype)
      && (rs6000_function_arg_padding (TYPE_MODE (valtype), valtype)
	  == PAD_UPWARD))
    return true;

  return false;

But, on the other hand, you should do this in rs6000_function_arg_padding
instead I think.

-msvr4-struct-return
-Target Report RejectNegative Var(aix_struct_return,0) Save
-Return small structures in registers (SVR4 default).
+msvr4-struct-return=
+Target RejectNegative Joined Enum(rs6000_svr4_struct_return)  Var(rs6000_current_svr4_struct_return)
+-msvr4-struct-return=[standard,gnu] Return small structures in registers (SVR4 default).
+
+Enum
+Name(rs6000_svr4_struct_return) Type(enum rs6000_svr4_struct_return)
+
+EnumValue
+Enum(rs6000_svr4_struct_return) String(standard) Value(SVR4_STRUCT_RETURN_STD) 
+
+EnumValue
+Enum(rs6000_svr4_struct_return) String(gnu) Value(SVR4_STRUCT_RETURN_GNU) 

You are removing the -msvr4-struct-return option (without =).  We shouldn't
delete command line options.

--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2018-12-06  Lokesh Janghel  <lokeshjanghel91@gmail.com>
+
+        PR target/84762
+        * gcc.target/pr84762-1.c: New testcase.
+        * gcc.target/pr84762-2.c: New testcase.
+        * gcc.target/pr84762-3.c: New testcase.

These should be before the patch as well.

+++ b/gcc/testsuite/gcc.target/powerpc/pr84762-1.c
@@ -0,0 +1,9 @@
+/* { dg-do run { target powerpc*-*-* rs6000-*-* } } */

You don't have to mention the target in gcc.target/powerpc; just say

+/* { dg-do run } */

+struct smallstruct { char a; char b; char c; };
+
+struct smallstruct f(void)
+{
+  struct smallstruct s = { 0x11, 0x22, 0x33 };
+
+  return s;
+}

And this cannot run anyway, it isn't a full program.  How was this tested?


+++ b/gcc/testsuite/gcc.target/powerpc/pr84762-2.c

+/* { dg-final { scan-assembler-times "lis \[\n\]*,0x\[\n\]*" 1 } } */
+/* { dg-final { scan-assembler-times "ori \[\n\]*,\[\n\]*,0x\[\n\]*" 1 } } */

These regexps can never match; they match invalid assembler code only.
Any scan-assembler regexp ending in a star is suspect btw (the star and
the atom before it can always be removed and it will still mean the same
thing).


Segher

Re: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84762

[Umesh Kalappa]

Thank you Jakub for the information.

Will make a note of it.

Umesh



On Tue, Dec 11, 2018, 17:58 Jakub Jelinek <jakub@redhat.com wrote:

> On Tue, Dec 11, 2018 at 05:30:48PM +0530, Umesh Kalappa wrote:
> > Hi All,
> >
> > Please find the attached patch for the subjected issue .
> >
> > Do please let me know your thoughts and comments on the same .
>
> Not a patch review (will defer that to rs6000 maintainers), but
> some comments on gcc-patches patch submissions.
>
> The subject should ideally start with [PATCH] or similar,
> then have some short summary of what the patch is about and if
> it fixes some PR, just PR something/12345 reference,
> the subjects you are posting like:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84762
> don't say anything relevant except for the PR 84762 number,
> so anyone reading gcc-patches needs to open that bug in order to even find
> out if it is something for him or somebody else.
>
> If you are sending a patch for an area that has some maintainer(s),
> usually you should either mention those maintainers in To: (and CC:
> gcc-patches) or To: gcc-patches, CC: the maintainers, to draw their
> attention.  See MAINTAINERS file in GCC tree.
>
> The mail body should start with a short explanation of what the problem is
> and how are you solving it, again, so that people don't have to jump to
> bugzilla to find out (of course, short is enough, no need to duplicate
> dozens of comments from the PR), should include information on what
> target(s) it has been bootstrapped/regtested.  And, it is always better if
> it is the patch author that posts it, or is at least CCed so that he can
> answer review questions.
>
> Thanks.
>
>         Jakub
>

Re: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84762

[Umesh Kalappa]

Thank you Segher, will work on your suggestions.

Umesh

On Tue, Dec 11, 2018, 19:23 Segher Boessenkool <segher@kernel.crashing.org
wrote:

> Hi Umesh,
>
> On Tue, Dec 11, 2018 at 05:30:48PM +0530, Umesh Kalappa wrote:
> > Please find the attached patch for the subjected issue .
> >
> > Do please let me know your thoughts and comments on the same .
>
> First of all: do you have a copyright assignment with the FSF?
>
> Second: please don't send application/octet-stream attachments.
>
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index ee5f183..d1c0edb 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,14 @@
> +2018-12-06  Lokesh Janghel  <lokeshjanghel91@gmail.com>
> +
> +        PR target/84762
> +        * config/rs6000/rs6000.c (rs6000_return_in_msb): Retrun in svr4
> for
> +        small struct value.
> +        (rs6000_option_override_internal): Modify the condition for aix or
> +        svr4.
> +       * config/rs6000/rs6000.opt : Modify the -msvr4-struct-return
> option.
> +       * config/rs6000/rs6000-opts.h : Add enum for svr4 option (Big
> endian
> +       and Little endian).
>
> The changelog should not be part of the patch, but written before it.
> Not as diff, just as the text it is.
>
> Indent is one tab.  Not a tab and a space, not nine spaces.
>
> There shouldn't be trailing spaces.
>
> There should not be a space before a colon.
>
> "Modify XYZ." means that you should have "(XYZ): Modify." instead; but
> you probably can say more than just "Modify", too.  Like, _what_ have you
> changed about it :-)
>
> s/retrun/return/
>
> The changelog should mention everything you change.  I haven't checked
> if it does here, but all the testcases are missing (those have their own
> changelog, in gcc/testsuite/ChangeLog).
>
> +/* Return small structs in register,
> +   gnu: LSB-aligned,
> +   standard: MSB-aligned    */
>
> This should end with dot space space */
>
> +enum rs6000_svr4_struct_return {
> +  SVR4_STRUCT_RETURN_GNU=1,
> +  SVR4_STRUCT_RETURN_STD
> +};
>
> I think a simple boolean would be easier?
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index 2765263..4751b61 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -4632,7 +4632,8 @@ rs6000_option_override_internal (bool global_init_p)
>        /* Set aix_struct_return last, after the ABI is determined.
>          If -maix-struct-return or -msvr4-struct-return was explicitly
>          used, don't override with the ABI default.  */
> -      if (!global_options_set.x_aix_struct_return)
> +      if (!global_options_set.x_aix_struct_return
> +          && !rs6000_current_svr4_struct_return)
>         aix_struct_return = (DEFAULT_ABI != ABI_V4 || DRAFT_V4_STRUCT_RET);
>
> Why this change?
>
>  static bool
>  rs6000_return_in_msb (const_tree valtype)
>  {
> -  return (DEFAULT_ABI == ABI_ELFv2
> -         && BYTES_BIG_ENDIAN
> +  return ((DEFAULT_ABI == ABI_ELFv2
> +          || (DEFAULT_ABI == ABI_V4
> +          && rs6000_current_svr4_struct_return == SVR4_STRUCT_RETURN_STD))
> +         && BYTES_BIG_ENDIAN
>           && AGGREGATE_TYPE_P (valtype)
>           && (rs6000_function_arg_padding (TYPE_MODE (valtype), valtype)
>               == PAD_UPWARD));
>
> Indents are with tabs, not eight spaces.  There never should be tabs
> after spaces though.
>
> Please write this as
>
>   if (DEFAULT_ABI == ABI_ELFv2
>       && BYTES_BIG_ENDIAN
>       && AGGREGATE_TYPE_P (valtype)
>       && (rs6000_function_arg_padding (TYPE_MODE (valtype), valtype)
>           == PAD_UPWARD))
>     return true;
>
>   if (DEFAULT_ABI == ABI_V4
>       && rs6000_current_svr4_struct_return == SVR4_STRUCT_RETURN_STD
>       && BYTES_BIG_ENDIAN
>       && AGGREGATE_TYPE_P (valtype)
>       && (rs6000_function_arg_padding (TYPE_MODE (valtype), valtype)
>           == PAD_UPWARD))
>     return true;
>
>   return false;
>
> But, on the other hand, you should do this in rs6000_function_arg_padding
> instead I think.
>
> -msvr4-struct-return
> -Target Report RejectNegative Var(aix_struct_return,0) Save
> -Return small structures in registers (SVR4 default).
> +msvr4-struct-return=
> +Target RejectNegative Joined Enum(rs6000_svr4_struct_return)
> Var(rs6000_current_svr4_struct_return)
> +-msvr4-struct-return=[standard,gnu] Return small structures in registers
> (SVR4 default).
> +
> +Enum
> +Name(rs6000_svr4_struct_return) Type(enum rs6000_svr4_struct_return)
> +
> +EnumValue
> +Enum(rs6000_svr4_struct_return) String(standard)
> Value(SVR4_STRUCT_RETURN_STD)
> +
> +EnumValue
> +Enum(rs6000_svr4_struct_return) String(gnu) Value(SVR4_STRUCT_RETURN_GNU)
>
> You are removing the -msvr4-struct-return option (without =).  We shouldn't
> delete command line options.
>
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,10 @@
> +2018-12-06  Lokesh Janghel  <lokeshjanghel91@gmail.com>
> +
> +        PR target/84762
> +        * gcc.target/pr84762-1.c: New testcase.
> +        * gcc.target/pr84762-2.c: New testcase.
> +        * gcc.target/pr84762-3.c: New testcase.
>
> These should be before the patch as well.
>
> +++ b/gcc/testsuite/gcc.target/powerpc/pr84762-1.c
> @@ -0,0 +1,9 @@
> +/* { dg-do run { target powerpc*-*-* rs6000-*-* } } */
>
> You don't have to mention the target in gcc.target/powerpc; just say
>
> +/* { dg-do run } */
>
> +struct smallstruct { char a; char b; char c; };
> +
> +struct smallstruct f(void)
> +{
> +  struct smallstruct s = { 0x11, 0x22, 0x33 };
> +
> +  return s;
> +}
>
> And this cannot run anyway, it isn't a full program.  How was this tested?
>
>
> +++ b/gcc/testsuite/gcc.target/powerpc/pr84762-2.c
>
> +/* { dg-final { scan-assembler-times "lis \[\n\]*,0x\[\n\]*" 1 } } */
> +/* { dg-final { scan-assembler-times "ori \[\n\]*,\[\n\]*,0x\[\n\]*" 1 }
> } */
>
> These regexps can never match; they match invalid assembler code only.
> Any scan-assembler regexp ending in a star is suspect btw (the star and
> the atom before it can always be removed and it will still mean the same
> thing).
>
>
> Segher
>