diff mbox series

[v6,1/7] Honor TARGET_PROMOTE_PROTOTYPES during RTL expand

Message ID 20241204204800.798153-2-hjl.tools@gmail.com
State New
Headers
Series Correct outgoing integer argument promotion |

Commit Message

H.J. Lu Dec. 4, 2024, 8:47 p.m. UTC 
  Promote integer arguments smaller than int if TARGET_PROMOTE_PROTOTYPES
returns true.

	PR middle-end/112877
	* calls.c (initialize_argument_information): Promote small integer
	arguments if TARGET_PROMOTE_PROTOTYPES returns true.

gcc/testsuite/

	PR middle-end/112877
	* gfortran.dg/pr112877-1.f90: New test.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 gcc/calls.cc                             |  9 +++++++++
 gcc/testsuite/gfortran.dg/pr112877-1.f90 | 17 +++++++++++++++++
 2 files changed, 26 insertions(+)
 create mode 100644 gcc/testsuite/gfortran.dg/pr112877-1.f90

Comments

Richard Biener Dec. 6, 2024, 12:39 p.m. UTC | #1 
On Wed, Dec 4, 2024 at 9:48 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> Promote integer arguments smaller than int if TARGET_PROMOTE_PROTOTYPES
> returns true.

This is OK when 2/7 got no negative comments and Jeff doesn't have
further input here.

(I think 1/7+2/7 are a good improvement on their own given the latent bugfixing)

Also CCed Eric who might also know calls.c quite well.

Richard.

>         PR middle-end/112877
>         * calls.c (initialize_argument_information): Promote small integer
>         arguments if TARGET_PROMOTE_PROTOTYPES returns true.
>
> gcc/testsuite/
>
>         PR middle-end/112877
>         * gfortran.dg/pr112877-1.f90: New test.
>
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> ---
>  gcc/calls.cc                             |  9 +++++++++
>  gcc/testsuite/gfortran.dg/pr112877-1.f90 | 17 +++++++++++++++++
>  2 files changed, 26 insertions(+)
>  create mode 100644 gcc/testsuite/gfortran.dg/pr112877-1.f90
>
> diff --git a/gcc/calls.cc b/gcc/calls.cc
> index 8cf0f29b42c..78ead6fd4ed 100644
> --- a/gcc/calls.cc
> +++ b/gcc/calls.cc
> @@ -1374,6 +1374,11 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
>        }
>    }
>
> +  bool promote_p
> +    = targetm.calls.promote_prototypes (fndecl
> +                                       ? TREE_TYPE (fndecl)
> +                                       : fntype);
> +
>    /* I counts args in order (to be) pushed; ARGPOS counts in order written.  */
>    for (argpos = 0; argpos < num_actuals; i--, argpos++)
>      {
> @@ -1383,6 +1388,10 @@ initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
>        /* Replace erroneous argument with constant zero.  */
>        if (type == error_mark_node || !COMPLETE_TYPE_P (type))
>         args[i].tree_value = integer_zero_node, type = integer_type_node;
> +      else if (promote_p
> +              && INTEGRAL_TYPE_P (type)
> +              && TYPE_PRECISION (type) < TYPE_PRECISION (integer_type_node))
> +       type = integer_type_node;
>
>        /* If TYPE is a transparent union or record, pass things the way
>          we would pass the first field of the union or record.  We have
> diff --git a/gcc/testsuite/gfortran.dg/pr112877-1.f90 b/gcc/testsuite/gfortran.dg/pr112877-1.f90
> new file mode 100644
> index 00000000000..f5596f0d0ad
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/pr112877-1.f90
> @@ -0,0 +1,17 @@
> +! { dg-do compile }
> +! { dg-options "-Os" }
> +
> +program test
> +    use iso_c_binding, only: c_short
> +    interface
> +      subroutine foo(a) bind(c)
> +        import c_short
> +        integer(kind=c_short), intent(in), value :: a
> +      end subroutine foo
> +    end interface
> +    integer(kind=c_short) a(5);
> +    call foo (a(3))
> +end
> +
> +! { dg-final { scan-assembler "movswl\t10\\(%rsp\\), %edi" { target { { i?86-*-linux* i?86-*-gnu* x86_64-*-linux* x86_64-*-gnu* } && { ! ia32 } } } } }
> +! { dg-final { scan-assembler "movswl\t-14\\(%ebp\\), %eax" { target { { i?86-*-linux* i?86-*-gnu* x86_64-*-linux* x86_64-*-gnu* } && { ia32 } } } } }
> --
> 2.47.1
>
Jeff Law Dec. 7, 2024, 4:23 p.m. UTC | #2 
On 12/6/24 5:39 AM, Richard Biener wrote:
> On Wed, Dec 4, 2024 at 9:48 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> Promote integer arguments smaller than int if TARGET_PROMOTE_PROTOTYPES
>> returns true.
> 
> This is OK when 2/7 got no negative comments and Jeff doesn't have
> further input here.
Nothing specific.  I'm leery of the change, but that's as much a 
function of being into stage3 as anything.

Jeff
Richard Biener Dec. 8, 2024, 9:25 a.m. UTC | #3 
On Sat, Dec 7, 2024 at 5:23 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 12/6/24 5:39 AM, Richard Biener wrote:
> > On Wed, Dec 4, 2024 at 9:48 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> Promote integer arguments smaller than int if TARGET_PROMOTE_PROTOTYPES
> >> returns true.
> >
> > This is OK when 2/7 got no negative comments and Jeff doesn't have
> > further input here.
> Nothing specific.  I'm leery of the change, but that's as much a
> function of being into stage3 as anything.

I have no strong opinion here as I think it should be pretty safe (and
it fixes a wrong-code
bug).  But I think we can also live with pushing this to next stage1
if HJ is fine with it.

Richard.

>
> Jeff
H.J. Lu Dec. 8, 2024, 11:16 a.m. UTC | #4 
On Sun, Dec 8, 2024, 5:25 PM Richard Biener <richard.guenther@gmail.com>
wrote:

> On Sat, Dec 7, 2024 at 5:23 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
> >
> >
> >
> > On 12/6/24 5:39 AM, Richard Biener wrote:
> > > On Wed, Dec 4, 2024 at 9:48 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >>
> > >> Promote integer arguments smaller than int if
> TARGET_PROMOTE_PROTOTYPES
> > >> returns true.
> > >
> > > This is OK when 2/7 got no negative comments and Jeff doesn't have
> > > further input here.
> > Nothing specific.  I'm leery of the change, but that's as much a
> > function of being into stage3 as anything.
>
> I have no strong opinion here as I think it should be pretty safe (and
> it fixes a wrong-code
> bug).  But I think we can also live with pushing this to next stage1
> if HJ is fine with it.
>

It can wait for GCC 16.

Thanks.


> Richard.
>
> >
> > Jeff
>
>
diff mbox series

Patch

diff --git a/gcc/calls.cc b/gcc/calls.cc
index 8cf0f29b42c..78ead6fd4ed 100644
--- a/gcc/calls.cc
+++ b/gcc/calls.cc
@@ -1374,6 +1374,11 @@  initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
       }
   }
 
+  bool promote_p
+    = targetm.calls.promote_prototypes (fndecl
+					? TREE_TYPE (fndecl)
+					: fntype);
+
   /* I counts args in order (to be) pushed; ARGPOS counts in order written.  */
   for (argpos = 0; argpos < num_actuals; i--, argpos++)
     {
@@ -1383,6 +1388,10 @@  initialize_argument_information (int num_actuals ATTRIBUTE_UNUSED,
       /* Replace erroneous argument with constant zero.  */
       if (type == error_mark_node || !COMPLETE_TYPE_P (type))
 	args[i].tree_value = integer_zero_node, type = integer_type_node;
+      else if (promote_p
+	       && INTEGRAL_TYPE_P (type)
+	       && TYPE_PRECISION (type) < TYPE_PRECISION (integer_type_node))
+	type = integer_type_node;
 
       /* If TYPE is a transparent union or record, pass things the way
 	 we would pass the first field of the union or record.  We have
diff --git a/gcc/testsuite/gfortran.dg/pr112877-1.f90 b/gcc/testsuite/gfortran.dg/pr112877-1.f90
new file mode 100644
index 00000000000..f5596f0d0ad
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr112877-1.f90
@@ -0,0 +1,17 @@ 
+! { dg-do compile }
+! { dg-options "-Os" }
+
+program test
+    use iso_c_binding, only: c_short
+    interface
+      subroutine foo(a) bind(c)
+        import c_short
+        integer(kind=c_short), intent(in), value :: a
+      end subroutine foo
+    end interface
+    integer(kind=c_short) a(5);
+    call foo (a(3))
+end
+
+! { dg-final { scan-assembler "movswl\t10\\(%rsp\\), %edi" { target { { i?86-*-linux* i?86-*-gnu* x86_64-*-linux* x86_64-*-gnu* } && { ! ia32 } } } } }
+! { dg-final { scan-assembler "movswl\t-14\\(%ebp\\), %eax" { target { { i?86-*-linux* i?86-*-gnu* x86_64-*-linux* x86_64-*-gnu* } && { ia32 } } } } }