| Message ID | 1f914dfd-e5c4-46fd-a1b2-e0c2ad32da38@BAMAIL02.ba.imgtec.org |
|---|---|
| State | New |
| Headers | show |
"Steve Ellcey " <sellcey@imgtec.com> writes: > MIPS architectures set TARGET_PROMOTE_PROTOTYPES to true. I would like > to have an option to set this to false in order to avoid extra masking > when passing char or short types. I don't think we can change this by > default since it would affect the ABI, but I would like to allow users > the option of turning if off if desired. > > Tested on mips-mti-elf. OK for checkin? > > Steve Ellcey > sellcey@imgtec.com > > > 2013-05-02 Steve Ellcey <sellcey@imgtec.com> > > * config/mips/mips.c (mips_promote_prototypes) :New. > (TARGET_PROMOTE_PROTOTYPES): Change to use mips_promote_prototypes. > * config/mips/mips.opt (mpromote-prototypes): New. It'd need an invoke.texi change too. The ABI thing is a problem though. Unlike the recent -mimadd option, this isn't something a user could reasonably turn on and off for individual files to see what happens. They'd need to rebuild all their libraries with it. And as written, the patch provides no way to compile gcc's own libraries that way, so they'd need to patch the gcc sources locally. If you want to change the TARGET_PROMOTE_PROTOTYPES part of the ABI for mips*-mti-elf then that would be OK with me. It would be better done without a command-line option. I'm less keen on adding -mpromote-prototypes to mips*-mti-elf, both because of the large number of variations there already, and because continuing to have -mno-promote-prototypes multilibs would give the impression that the change isn't much of a win. Ideally we'd also have a .gnu_attribute to record which promotion rules are being used, so that the linker can pick up incompatibilities. Sorry to be a pain... Thanks, Richard
On Fri, May 3, 2013 at 12:26 AM, Richard Sandiford <rdsandiford@googlemail.com> wrote: > "Steve Ellcey " <sellcey@imgtec.com> writes: >> MIPS architectures set TARGET_PROMOTE_PROTOTYPES to true. I would like >> to have an option to set this to false in order to avoid extra masking >> when passing char or short types. I don't think we can change this by >> default since it would affect the ABI, but I would like to allow users >> the option of turning if off if desired. >> >> Tested on mips-mti-elf. OK for checkin? >> >> Steve Ellcey >> sellcey@imgtec.com >> >> >> 2013-05-02 Steve Ellcey <sellcey@imgtec.com> >> >> * config/mips/mips.c (mips_promote_prototypes) :New. >> (TARGET_PROMOTE_PROTOTYPES): Change to use mips_promote_prototypes. >> * config/mips/mips.opt (mpromote-prototypes): New. > > It'd need an invoke.texi change too. > > The ABI thing is a problem though. Unlike the recent -mimadd option, > this isn't something a user could reasonably turn on and off for > individual files to see what happens. They'd need to rebuild all > their libraries with it. And as written, the patch provides no way > to compile gcc's own libraries that way, so they'd need to patch the > gcc sources locally. > > If you want to change the TARGET_PROMOTE_PROTOTYPES part of the > ABI for mips*-mti-elf then that would be OK with me. It would > be better done without a command-line option. > > I'm less keen on adding -mpromote-prototypes to mips*-mti-elf, > both because of the large number of variations there already, > and because continuing to have -mno-promote-prototypes multilibs > would give the impression that the change isn't much of a win. > > Ideally we'd also have a .gnu_attribute to record which promotion > rules are being used, so that the linker can pick up incompatibilities. > > Sorry to be a pain... An alternative approach would be to change the default for local, non-exported functions only. Similar to how the i386 backend chooses different argument passing conventions if it doesn't affect the ABI. You'd see the most effect when using -flto only, of course. Richard. > Thanks, > Richard
On Thu, 2013-05-02 at 23:26 +0100, Richard Sandiford wrote: > > > > 2013-05-02 Steve Ellcey <sellcey@imgtec.com> > > > > * config/mips/mips.c (mips_promote_prototypes) :New. > > (TARGET_PROMOTE_PROTOTYPES): Change to use mips_promote_prototypes. > > * config/mips/mips.opt (mpromote-prototypes): New. > > It'd need an invoke.texi change too. > > The ABI thing is a problem though. Unlike the recent -mimadd option, > this isn't something a user could reasonably turn on and off for > individual files to see what happens. They'd need to rebuild all > their libraries with it. And as written, the patch provides no way > to compile gcc's own libraries that way, so they'd need to patch the > gcc sources locally. I wasn't looking at providing an extra set of libraries, my assumption was that the libraries would be compiled with the default setting of promoting prototypes they way they are now and that these would still work with objects that were compiled with -mno-promote-prototypes. > If you want to change the TARGET_PROMOTE_PROTOTYPES part of the > ABI for mips*-mti-elf then that would be OK with me. It would > be better done without a command-line option. I don't think I want to change the ABI for mips*-mti-elf at this point. > I'm less keen on adding -mpromote-prototypes to mips*-mti-elf, > both because of the large number of variations there already, > and because continuing to have -mno-promote-prototypes multilibs > would give the impression that the change isn't much of a win. I definitely don't want to create another set of multilibs. > > Ideally we'd also have a .gnu_attribute to record which promotion > rules are being used, so that the linker can pick up incompatibilities. > > Sorry to be a pain... > > Thanks, > Richard That's alright, I will drop this idea for now and perhaps look at Richard Biener's idea (optimizing out the promotion on local calls) later. Steve Ellcey sellcey@imgtec.com
Steve Ellcey <sellcey@imgtec.com> writes: > On Thu, 2013-05-02 at 23:26 +0100, Richard Sandiford wrote: >> > 2013-05-02 Steve Ellcey <sellcey@imgtec.com> >> > >> > * config/mips/mips.c (mips_promote_prototypes) :New. >> > (TARGET_PROMOTE_PROTOTYPES): Change to use mips_promote_prototypes. >> > * config/mips/mips.opt (mpromote-prototypes): New. >> >> It'd need an invoke.texi change too. >> >> The ABI thing is a problem though. Unlike the recent -mimadd option, >> this isn't something a user could reasonably turn on and off for >> individual files to see what happens. They'd need to rebuild all >> their libraries with it. And as written, the patch provides no way >> to compile gcc's own libraries that way, so they'd need to patch the >> gcc sources locally. > > I wasn't looking at providing an extra set of libraries, my assumption > was that the libraries would be compiled with the default setting of > promoting prototypes they way they are now and that these would still > work with objects that were compiled with -mno-promote-prototypes. The problem is that TARGET_PROMOTE_PROTOTYPES is both a requirement on the caller and a guarantee to the callee. Any library function that takes a sub-int type is allowed to assume that the type is passed in extended form, and might be making use of that. And typedefs can make it less obvious which types are sub-int and which aren't. >> Ideally we'd also have a .gnu_attribute to record which promotion >> rules are being used, so that the linker can pick up incompatibilities. >> >> Sorry to be a pain... >> >> Thanks, >> Richard > > That's alright, I will drop this idea for now and perhaps look at > Richard Biener's idea (optimizing out the promotion on local calls) > later. Thanks, going with Richard's suggestion sounds good. Richard
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index 7545b60..072a68b 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -5721,6 +5721,15 @@ mips_return_in_memory (const_tree type, const_tree fndecl ATTRIBUTE_UNUSED) ? TYPE_MODE (type) == BLKmode : !IN_RANGE (int_size_in_bytes (type), 0, 2 * UNITS_PER_WORD)); } + +/* Implement TARGET_PROMOTE_PROTOTYPES. It is true by default but can + be turned off with -mno-promote-prototypes. */ + +static bool +mips_promote_prototypes (const_tree t ATTRIBUTE_UNUSED) +{ + return (mips_do_promote_prototypes != 0); +} /* Implement TARGET_SETUP_INCOMING_VARARGS. */ @@ -18708,7 +18717,7 @@ mips_expand_vec_minmax (rtx target, rtx op0, rtx op1, #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_PROTOTYPES mips_promote_prototypes #undef TARGET_FUNCTION_VALUE #define TARGET_FUNCTION_VALUE mips_function_value diff --git a/gcc/config/mips/mips.opt b/gcc/config/mips/mips.opt index e11710d..1881d9d 100644 --- a/gcc/config/mips/mips.opt +++ b/gcc/config/mips/mips.opt @@ -305,6 +305,10 @@ mpaired-single Target Report Mask(PAIRED_SINGLE_FLOAT) Use paired-single floating-point instructions +mpromote-prototypes +Target Report Var(mips_do_promote_prototypes) Init(1) +Promote integral arguments smaller then an int to int. + mr10k-cache-barrier= Target Joined RejectNegative Enum(mips_r10k_cache_barrier_setting) Var(mips_r10k_cache_barrier) Init(R10K_CACHE_BARRIER_NONE) -mr10k-cache-barrier=SETTING Specify when r10k cache barriers should be inserted