diff mbox

[MIPS] Remove definition of TARGET_PROMOTE_PROTOTYPES

Message ID 9003b977-ba2d-4c32-9078-af164c7305c8@BAMAIL02.ba.imgtec.org
State New
Headers show

Commit Message

Steve Ellcey Nov. 10, 2015, 11:57 p.m. UTC
This patch removes the definition of TARGET_PROMOTE_PROTOTYPES from MIPS,
where it was defined as true, so that it now defaults to false.

Currently MIPS does prototype promotion in the caller and the callee and this
patch removes the TARGET_PROMOTE_PROTOTYPES macro definition so that
the promotion is only done in the caller (due to PROMOTE_MODE being defined).
This does not break the ABI which requires the caller to do promotions anyway.
(See https://gcc.gnu.org/ml/gcc/2015-10/msg00223.html).  This change also
causes GCC to match what the LLVM and Greenhills compilers already do on MIPS.

After removing this macro I had three regressions, two were just tests that
needed changing but one was a bug (gcc.dg/fixed-point/convert-sat.c).
This test was calling a library function to convert a signed char into an
unsigned fixed type and because we don't have tree type information about
libcalls GCC cannot do the ABI required type promotion on those calls that it
does on normal user defined calls.  In fact promote_mode in explow.c expicitly
returns without doing anything if no type is given it.  Before this change it
didn't matter on MIPS because the callee did the same promotion that the caller
was supposed to have done before using the argument.  Now that callee code is
gone we depend on the caller doing the correct promotion and that was not
happening.

I submitted and checked in another patch to optabs.c
(See https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00704.html) to provide
me with the infrastructure to do the correct type promotion in expand_fixed
and this patch redefines TARGET_PROMOTE_FUNCTION_MODE to return the needed
promotion mode even when type is NULL_TREE.  When type is set it does
the same thing as it used to do.  This change allows me to remote the
definition of TARGET_PROMOTE_PROTOTYPES without the convert-sat.c test
failing.

The two tests that I changed are gcc.dg/tree-ssa/ssa-fre-4.c and
gcc.target/mips/ext-2.c.  ssa-fre-4.c no longer applies to MIPS now
that we do not define TARGET_PROMOTE_PROTOTYPES so I removed the MIPS
target from it.  ext-2.c now generates an srl instruction instead of a
dext instruction but the number of instructions has not changed and I
updated the scan checks.

Tested on mips-mti-linux-gnu with no unfixed regressions.  OK to checkin?

Steve Ellcey
sellcey@imgtec.com


2015-11-10  Steve Ellcey  <sellcey@imgtec.com>

	* config/mips/mips.c (mips_promote_function_mode): New function.
	(TARGET_PROMOTE_FUNCTION_MODE): Define as above function.
	(TARGET_PROMOTE_PROTOTYPES): Remove.

Comments

Steve Ellcey Dec. 11, 2015, 11:04 p.m. UTC | #1
Patch ping.

Steve Ellcey
sellcey@imgtec.com


On Tue, 2015-11-10 at 15:57 -0800, Steve Ellcey wrote:
> This patch removes the definition of TARGET_PROMOTE_PROTOTYPES from MIPS,
> where it was defined as true, so that it now defaults to false.
> 
> Currently MIPS does prototype promotion in the caller and the callee and this
> patch removes the TARGET_PROMOTE_PROTOTYPES macro definition so that
> the promotion is only done in the caller (due to PROMOTE_MODE being defined).
> This does not break the ABI which requires the caller to do promotions anyway.
> (See https://gcc.gnu.org/ml/gcc/2015-10/msg00223.html).  This change also
> causes GCC to match what the LLVM and Greenhills compilers already do on MIPS.
> 
> After removing this macro I had three regressions, two were just tests that
> needed changing but one was a bug (gcc.dg/fixed-point/convert-sat.c).
> This test was calling a library function to convert a signed char into an
> unsigned fixed type and because we don't have tree type information about
> libcalls GCC cannot do the ABI required type promotion on those calls that it
> does on normal user defined calls.  In fact promote_mode in explow.c expicitly
> returns without doing anything if no type is given it.  Before this change it
> didn't matter on MIPS because the callee did the same promotion that the caller
> was supposed to have done before using the argument.  Now that callee code is
> gone we depend on the caller doing the correct promotion and that was not
> happening.
> 
> I submitted and checked in another patch to optabs.c
> (See https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00704.html) to provide
> me with the infrastructure to do the correct type promotion in expand_fixed
> and this patch redefines TARGET_PROMOTE_FUNCTION_MODE to return the needed
> promotion mode even when type is NULL_TREE.  When type is set it does
> the same thing as it used to do.  This change allows me to remote the
> definition of TARGET_PROMOTE_PROTOTYPES without the convert-sat.c test
> failing.
> 
> The two tests that I changed are gcc.dg/tree-ssa/ssa-fre-4.c and
> gcc.target/mips/ext-2.c.  ssa-fre-4.c no longer applies to MIPS now
> that we do not define TARGET_PROMOTE_PROTOTYPES so I removed the MIPS
> target from it.  ext-2.c now generates an srl instruction instead of a
> dext instruction but the number of instructions has not changed and I
> updated the scan checks.
> 
> Tested on mips-mti-linux-gnu with no unfixed regressions.  OK to checkin?
> 
> Steve Ellcey
> sellcey@imgtec.com
> 
> 
> 2015-11-10  Steve Ellcey  <sellcey@imgtec.com>
> 
> 	* config/mips/mips.c (mips_promote_function_mode): New function.
> 	(TARGET_PROMOTE_FUNCTION_MODE): Define as above function.
> 	(TARGET_PROMOTE_PROTOTYPES): Remove.
> 
> 
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index 9880b23..e9c3830 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -19760,6 +19760,32 @@ mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
>      return GR_REGS;
>    return allocno_class;
>  }
> +
> +/* Implement TARGET_PROMOTE_FUNCTION_MODE */
> +
> +/* This function is equivalent to default_promote_function_mode_always_promote
> +   except that it returns a promoted mode even if type is NULL_TREE.  This is
> +   needed by libcalls which have no type (only a mode) such as fixed conversion
> +   routines that take a signed or unsigned char/short argument and convert it
> +   to a fixed type.  */
> +
> +static machine_mode
> +mips_promote_function_mode (const_tree type ATTRIBUTE_UNUSED,
> +                            machine_mode mode,
> +                            int *punsignedp ATTRIBUTE_UNUSED,
> +                            const_tree fntype ATTRIBUTE_UNUSED,
> +                            int for_return ATTRIBUTE_UNUSED)
> +{
> +  int unsignedp;
> +
> +  if (type != NULL_TREE)
> +    return promote_mode (type, mode, punsignedp);
> +
> +  unsignedp = *punsignedp;
> +  PROMOTE_MODE (mode, unsignedp, type);
> +  *punsignedp = unsignedp;
> +  return mode;
> +}
>  
>  /* Initialize the GCC target structure.  */
>  #undef TARGET_ASM_ALIGNED_HI_OP
> @@ -19864,10 +19890,7 @@ mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
>  #define TARGET_GIMPLIFY_VA_ARG_EXPR mips_gimplify_va_arg_expr
>  
>  #undef  TARGET_PROMOTE_FUNCTION_MODE
> -#define TARGET_PROMOTE_FUNCTION_MODE default_promote_function_mode_always_promote
> -#undef TARGET_PROMOTE_PROTOTYPES
> -#define TARGET_PROMOTE_PROTOTYPES hook_bool_const_tree_true
> -
> +#define TARGET_PROMOTE_FUNCTION_MODE mips_promote_function_mode
>  #undef TARGET_FUNCTION_VALUE
>  #define TARGET_FUNCTION_VALUE mips_function_value
>  #undef TARGET_LIBCALL_VALUE
> 
> 
> 
> 
> 2015-11-10  Steve Ellcey  <sellcey@imgtec.com>
> 
> 	* gcc.dg/tree-ssa/ssa-fre-4.c: Remove mips*-*-* target.
> 	* gcc.target/mips/ext-2.c: Update scan checks.
> 
> 
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-4.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-4.c
> index 02b6719..5a7588f 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-4.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-4.c
> @@ -1,6 +1,6 @@
>  /* If the target returns false for TARGET_PROMOTE_PROTOTYPES, then there
>     will be no casts for FRE to eliminate and the test will fail.  */
> -/* { dg-do compile { target i?86-*-* x86_64-*-* hppa*-*-* mips*-*-* m68k*-*-* } } */
> +/* { dg-do compile { target i?86-*-* x86_64-*-* hppa*-*-* m68k*-*-* } } */
>  /* { dg-options "-O -fno-tree-ccp -fno-tree-forwprop -fdump-tree-fre1-details" } */
>  
>  /* From PR21608.  */
> diff --git a/gcc/testsuite/gcc.target/mips/ext-2.c b/gcc/testsuite/gcc.target/mips/ext-2.c
> index 320d42d..9770321 100644
> --- a/gcc/testsuite/gcc.target/mips/ext-2.c
> +++ b/gcc/testsuite/gcc.target/mips/ext-2.c
> @@ -1,12 +1,13 @@
>  /* Turn the truncate,zero_extend,lshiftrt sequence before the or into a
>     zero_extract.  The truncate is due to TARGET_PROMOTE_PROTOTYPES, the
> -   zero_extend to PROMOTE_MODE.  */
> +   zero_extend to PROMOTE_MODE.  As of GCC 6, we no longer define
> +   TARGET_PROMOTE_PROTOTYPES so that truncate is gone and we can
> +   generate either a single extract or a single shift instruction.  */
>  /* { dg-do compile } */
>  /* { dg-options "isa_rev>=2 -mgp64 -mlong64" } */
>  /* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
> -/* { dg-final { scan-assembler "\tdext\t" } } */
> +/* { dg-final { scan-assembler-times "\tdext\t|\td?srl" 1 } } */
>  /* { dg-final { scan-assembler-not "\tand" } } */
> -/* { dg-final { scan-assembler-not "\td?srl" } } */
>  
>  NOMIPS16 void
>  f (unsigned char x, unsigned char *r)
Matthew Fortune Dec. 12, 2015, 8:53 a.m. UTC | #2
Steve Ellcey <Steve.Ellcey@imgtec.com> writes:
> On Tue, 2015-11-10 at 15:57 -0800, Steve Ellcey wrote:
> > 2015-11-10  Steve Ellcey  <sellcey@imgtec.com>
> >
> > 	* config/mips/mips.c (mips_promote_function_mode): New function.
> > 	(TARGET_PROMOTE_FUNCTION_MODE): Define as above function.
> > 	(TARGET_PROMOTE_PROTOTYPES): Remove.

I'm OK with this change on the basis that MIPS has been providing stronger
guarantees than required by the various standards. I.e. after this change
MIPS will have undefined behaviour for a mismatch in types between a
call to an un-prototyped function and its definition:

extern void foo();

void caller(int a)
{
  foo(a);
}

--

void foo(short a)
{
  // the value of 'a' can be out of range of a short because the caller
  // did not get the right type for the argument.
}

Thanks,
Matthew
Maciej W. Rozycki Jan. 19, 2016, 3:26 p.m. UTC | #3
On Sat, 12 Dec 2015, Matthew Fortune wrote:

> > > 	* config/mips/mips.c (mips_promote_function_mode): New function.
> > > 	(TARGET_PROMOTE_FUNCTION_MODE): Define as above function.
> > > 	(TARGET_PROMOTE_PROTOTYPES): Remove.
> 
> I'm OK with this change on the basis that MIPS has been providing stronger
> guarantees than required by the various standards. I.e. after this change
> MIPS will have undefined behaviour for a mismatch in types between a
> call to an un-prototyped function and its definition:

 Indeed this is exactly what the current ISO C language standard mandates 
-- if an unprototyped call is made to a function whose definition has been 
prototyped and the types of the arguments after promotion are incompatible 
with the types of the respective parameters, then behaviour is undefined.

> extern void foo();
> 
> void caller(int a)
> {
>   foo(a);
> }
> 
> --
> 
> void foo(short a)
> {
>   // the value of 'a' can be out of range of a short because the caller
>   // did not get the right type for the argument.
> }

 Which is exactly the case with the piece of code you quoted.  Behaviour 
of this code would be defined if the `a' parameter of `foo' was of the 
`int' type.  See Section 6.5.2.2 "Function calls", clause 6, for details.

  Maciej
diff mbox

Patch

diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 9880b23..e9c3830 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -19760,6 +19760,32 @@  mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
     return GR_REGS;
   return allocno_class;
 }
+
+/* Implement TARGET_PROMOTE_FUNCTION_MODE */
+
+/* This function is equivalent to default_promote_function_mode_always_promote
+   except that it returns a promoted mode even if type is NULL_TREE.  This is
+   needed by libcalls which have no type (only a mode) such as fixed conversion
+   routines that take a signed or unsigned char/short argument and convert it
+   to a fixed type.  */
+
+static machine_mode
+mips_promote_function_mode (const_tree type ATTRIBUTE_UNUSED,
+                            machine_mode mode,
+                            int *punsignedp ATTRIBUTE_UNUSED,
+                            const_tree fntype ATTRIBUTE_UNUSED,
+                            int for_return ATTRIBUTE_UNUSED)
+{
+  int unsignedp;
+
+  if (type != NULL_TREE)
+    return promote_mode (type, mode, punsignedp);
+
+  unsignedp = *punsignedp;
+  PROMOTE_MODE (mode, unsignedp, type);
+  *punsignedp = unsignedp;
+  return mode;
+}
 
 /* Initialize the GCC target structure.  */
 #undef TARGET_ASM_ALIGNED_HI_OP
@@ -19864,10 +19890,7 @@  mips_ira_change_pseudo_allocno_class (int regno, reg_class_t allocno_class)
 #define TARGET_GIMPLIFY_VA_ARG_EXPR mips_gimplify_va_arg_expr
 
 #undef  TARGET_PROMOTE_FUNCTION_MODE
-#define TARGET_PROMOTE_FUNCTION_MODE default_promote_function_mode_always_promote
-#undef TARGET_PROMOTE_PROTOTYPES
-#define TARGET_PROMOTE_PROTOTYPES hook_bool_const_tree_true
-
+#define TARGET_PROMOTE_FUNCTION_MODE mips_promote_function_mode
 #undef TARGET_FUNCTION_VALUE
 #define TARGET_FUNCTION_VALUE mips_function_value
 #undef TARGET_LIBCALL_VALUE




2015-11-10  Steve Ellcey  <sellcey@imgtec.com>

	* gcc.dg/tree-ssa/ssa-fre-4.c: Remove mips*-*-* target.
	* gcc.target/mips/ext-2.c: Update scan checks.


diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-4.c b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-4.c
index 02b6719..5a7588f 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-4.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-4.c
@@ -1,6 +1,6 @@ 
 /* If the target returns false for TARGET_PROMOTE_PROTOTYPES, then there
    will be no casts for FRE to eliminate and the test will fail.  */
-/* { dg-do compile { target i?86-*-* x86_64-*-* hppa*-*-* mips*-*-* m68k*-*-* } } */
+/* { dg-do compile { target i?86-*-* x86_64-*-* hppa*-*-* m68k*-*-* } } */
 /* { dg-options "-O -fno-tree-ccp -fno-tree-forwprop -fdump-tree-fre1-details" } */
 
 /* From PR21608.  */
diff --git a/gcc/testsuite/gcc.target/mips/ext-2.c b/gcc/testsuite/gcc.target/mips/ext-2.c
index 320d42d..9770321 100644
--- a/gcc/testsuite/gcc.target/mips/ext-2.c
+++ b/gcc/testsuite/gcc.target/mips/ext-2.c
@@ -1,12 +1,13 @@ 
 /* Turn the truncate,zero_extend,lshiftrt sequence before the or into a
    zero_extract.  The truncate is due to TARGET_PROMOTE_PROTOTYPES, the
-   zero_extend to PROMOTE_MODE.  */
+   zero_extend to PROMOTE_MODE.  As of GCC 6, we no longer define
+   TARGET_PROMOTE_PROTOTYPES so that truncate is gone and we can
+   generate either a single extract or a single shift instruction.  */
 /* { dg-do compile } */
 /* { dg-options "isa_rev>=2 -mgp64 -mlong64" } */
 /* { dg-skip-if "code quality test" { *-*-* } { "-O0" } { "" } } */
-/* { dg-final { scan-assembler "\tdext\t" } } */
+/* { dg-final { scan-assembler-times "\tdext\t|\td?srl" 1 } } */
 /* { dg-final { scan-assembler-not "\tand" } } */
-/* { dg-final { scan-assembler-not "\td?srl" } } */
 
 NOMIPS16 void
 f (unsigned char x, unsigned char *r)