Patchwork [mips] Allow users to avoid promoting prototypes.

login
register
mail settings
Submitter Steve Ellcey
Date May 2, 2013, 8:12 p.m.
Message ID <1f914dfd-e5c4-46fd-a1b2-e0c2ad32da38@BAMAIL02.ba.imgtec.org>
Download mbox | patch
Permalink /patch/241078/
State New
Headers show

Comments

Steve Ellcey - May 2, 2013, 8:12 p.m.
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.
Richard Sandiford - May 2, 2013, 10:26 p.m.
"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
Richard Guenther - May 3, 2013, 8:05 a.m.
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
Steve Ellcey - May 3, 2013, 4:21 p.m.
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
Richard Sandiford - May 4, 2013, 9:23 a.m.
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

Patch

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