Message ID | 9003b977-ba2d-4c32-9078-af164c7305c8@BAMAIL02.ba.imgtec.org |
---|---|
State | New |
Headers | show |
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)
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
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 --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)