[RFD] MIPS/gas: Optimisation cannot be set to 0

[RFD] MIPS/gas: Optimisation cannot be set to 0

[Maciej W. Rozycki]

Hello,

 Browsing through gas code I have noticed this snippet:

/* Whether we are optimizing.  The default value of 2 means to remove
   unneeded NOPs and swap branch instructions when possible.  A value
   of 1 means to not swap branches.  A value of 0 means to always
   insert NOPs.  */
static int mips_optimize = 2;

and all the three values are checked against throughout.  However, it is 
not possible to set the variable to 0 -- the only piece of code doing 
initialisation is this:

    case 'O':
      if (arg && arg[0] == '0')
	mips_optimize = 1;
      else
	mips_optimize = 2;
      break;

in md_parse_option().  It looks like it has been like this since our CVS 
tree was born.  There is this ChangeLog entry which may be of relevance 
though:

Wed Feb 12 14:36:29 1997  Ian Lance Taylor  <ian@cygnus.com>

	* config/tc-mips.c (md_parse_option): When debugging, set
	mips_optimize to 1, not 0.

 Anyway, the outcome is we have dead code and unclear semantics.  The most 
obvious solution is mapping the argument of -O directly to the value of 
mips_optimize.  The drawback is changed semantics.  Another possibility is 
getting rid of what currently mips_optimize == 1 means entirely.  And 
there may be more reasonable options available.

 I have no strong preference, but I think gas should have a mode where its 
input is assembled intact.  And swapping branches may produce surprising 
interactions with debugging information.  I am therefore in favour to the 
first proposal above, but I would like to hear from the others.

  Maciej

Re: [RFD] MIPS/gas: Optimisation cannot be set to 0

[Thiemo Seufer]

Maciej W. Rozycki wrote:
> Hello,
> 
>  Browsing through gas code I have noticed this snippet:
> 
> /* Whether we are optimizing.  The default value of 2 means to remove
>    unneeded NOPs and swap branch instructions when possible.  A value
>    of 1 means to not swap branches.  A value of 0 means to always
>    insert NOPs.  */
> static int mips_optimize = 2;
> 
> and all the three values are checked against throughout.  However, it is 
> not possible to set the variable to 0 -- the only piece of code doing 
> initialisation is this:
> 
>     case 'O':
>       if (arg && arg[0] == '0')
> 	mips_optimize = 1;
>       else
> 	mips_optimize = 2;
>       break;
> 
> in md_parse_option().  It looks like it has been like this since our CVS 
> tree was born.  There is this ChangeLog entry which may be of relevance 
> though:
> 
> Wed Feb 12 14:36:29 1997  Ian Lance Taylor  <ian@cygnus.com>
> 
> 	* config/tc-mips.c (md_parse_option): When debugging, set
> 	mips_optimize to 1, not 0.
> 
>  Anyway, the outcome is we have dead code and unclear semantics.  The most 
> obvious solution is mapping the argument of -O directly to the value of 
> mips_optimize.  The drawback is changed semantics.  Another possibility is 
> getting rid of what currently mips_optimize == 1 means entirely.  And 
> there may be more reasonable options available.
> 
>  I have no strong preference, but I think gas should have a mode where its 
> input is assembled intact.  And swapping branches may produce surprising 
> interactions with debugging information.  I am therefore in favour to the 
> first proposal above, but I would like to hear from the others.

I also prefer the first option, given that gcc carefully avoids to pass
-O0 in most cases:

#ifndef SUBTARGET_ASM_OPTIMIZING_SPEC
#define SUBTARGET_ASM_OPTIMIZING_SPEC "\
%{noasmopt:-O0} \
%{!noasmopt:%{O:-O2} %{O1:-O2} %{O2:-O2} %{O3:-O3}}"
#endif

The noasmopt option is undocumented.


Thiemo

Re: [RFD] MIPS/gas: Optimisation cannot be set to 0

[Ian Lance Taylor]

"Maciej W. Rozycki" <macro@mips.com> writes:

> Wed Feb 12 14:36:29 1997  Ian Lance Taylor  <ian@cygnus.com>
> 
> 	* config/tc-mips.c (md_parse_option): When debugging, set
> 	mips_optimize to 1, not 0.

My vague recollection of the problem is that at least at that time gcc
would always pass a -O option to the assembler.  When not optimizing,
it would pass -O0.  However, inserting NOPs for MIPS variants which do
not need them does not help debugging.

Clearly failing to provide a way to set mips_optimize to 0 was an
oversight.

>  I have no strong preference, but I think gas should have a mode where its 
> input is assembled intact.  And swapping branches may produce surprising 
> interactions with debugging information.  I am therefore in favour to the 
> first proposal above, but I would like to hear from the others.

I'm not sure I see the relevance of "assembled intact."  The source is
assembled just as the user specified with -O1.  -O1 simply inhibits
adding additional unnecessary nop instructions--nop instructions that
were required for MIPS ISA I but not for later ISAs.

At -O1 branches are not swapped, so debugging is not impaired.

I agree there should be a way to set mips_optimize to 0, but we should
ensure that gcc -O0 does not set mips_optimize to 0 when assembling.

Ian

Re: [RFD] MIPS/gas: Optimisation cannot be set to 0

[Maciej W. Rozycki]

On Fri, 9 Nov 2007, Ian Lance Taylor wrote:

> My vague recollection of the problem is that at least at that time gcc
> would always pass a -O option to the assembler.  When not optimizing,
> it would pass -O0.  However, inserting NOPs for MIPS variants which do
> not need them does not help debugging.

 Presumably it does not impair it either.  It's no more the case as 
noticed by Thiemo though.

> I'm not sure I see the relevance of "assembled intact."  The source is
> assembled just as the user specified with -O1.  -O1 simply inhibits
> adding additional unnecessary nop instructions--nop instructions that
> were required for MIPS ISA I but not for later ISAs.

 The "assembled intact" phrase comes from me just interpreting the comment 
as is -- I have had a look at the code and mips_optimize == 0 means not to 
remove NOPs inserted by gas itself, rather than optimising away 
unnecessary padding supplied by the source code (".set reorder" always 
assumed).  Sorry about the confusion.

> At -O1 branches are not swapped, so debugging is not impaired.
> 
> I agree there should be a way to set mips_optimize to 0, but we should
> ensure that gcc -O0 does not set mips_optimize to 0 when assembling.

 These days GCC is meant to handle delay slot scheduling itself, so 
perhaps the best bet is not to pass -O options from the driver to gas at 
all.  OTOH, apparently -mips16 still does not schedule slots and uses 
".set reorder" at least for some configurations and this branch swapping 
messes with debugging terribly -- it's no fun to have a breakpoint set on 
the second half of an exteded branch/jump instruction. :-(  Which is why I 
have had a look at gas in this area in the first place.

  Maciej

Re: [RFD] MIPS/gas: Optimisation cannot be set to 0

[Paul Koning]

>>>>> "Ian" == Ian Lance Taylor <iant@google.com> writes:

 Ian> "Maciej W. Rozycki" <macro@mips.com> writes:
 >> Wed Feb 12 14:36:29 1997 Ian Lance Taylor <ian@cygnus.com>
 >> 
 >> * config/tc-mips.c (md_parse_option): When debugging, set
 >> mips_optimize to 1, not 0.

 Ian> My vague recollection of the problem is that at least at that
 Ian> time gcc would always pass a -O option to the assembler.  When
 Ian> not optimizing, it would pass -O0.  However, inserting NOPs for
 Ian> MIPS variants which do not need them does not help debugging.

 Ian> Clearly failing to provide a way to set mips_optimize to 0 was
 Ian> an oversight.

I thought that GCC these days (as of V3.4 or so) tells the assembler
not to do any of those silly transformations -- instead the compiler
does it, because the compiler's code generator has a lot more
information and can do a much better job.

This is as it should be.  "optimizing assembler" is a contradiction in
terms.

In other words, I thought that gcc now tells the assembler ".set
noreorder; .set nomacro; .set noat" so the assembler isn't going to do
ANY transformations.

    paul

Re: [RFD] MIPS/gas: Optimisation cannot be set to 0

[Maciej W. Rozycki]

On Fri, 9 Nov 2007, Paul Koning wrote:

> This is as it should be.  "optimizing assembler" is a contradiction in
> terms.

 Agreed, but given some of the peculiarities of the MIPS processor, it is 
very useful for hand-coded assembly -- load-delay slots, dependencies 
between mfhi/mflo and the following multiply/divide instructions, etc. 
which vary across members of the family.  The output of GCC should need no 
optimisation in any case, no argument.

> In other words, I thought that gcc now tells the assembler ".set
> noreorder; .set nomacro; .set noat" so the assembler isn't going to do
> ANY transformations.

 Not for inline asms (understandable) and (to my surprise!) not for MIPS16 
code.

  Maciej

Re: [RFD] MIPS/gas: Optimisation cannot be set to 0

[Richard Sandiford]

"Maciej W. Rozycki" <macro@mips.com> writes:
> On Fri, 9 Nov 2007, Paul Koning wrote:
>> In other words, I thought that gcc now tells the assembler ".set
>> noreorder; .set nomacro; .set noat" so the assembler isn't going to do
>> ANY transformations.
>
>  Not for inline asms (understandable) and (to my surprise!) not for MIPS16 
> code.

Right.  There's not really any point doing it for MIPS16 code.  The only
kind of reordering you get is when GAS fills a delay slot that GCC didn't
know how to fill, so ".set noreorder" would mean "don't make this
function as small as you can".  That'd be a bit perverse on a target
where size is really the only thing that matters.  (In contrast,
GCC might have had good reasons to do what it did for non-MIPS16 code,
especially on targets like the VR413x.)

It would be quite hard for GCC to calculate precise (rather than
conversatively-correct) lengths for every MIPS16 instruction.
One of the problems is that the range of unextended PC-relative
instructions depends on the low bits of the instruction's address,
and GCC's length calculation code isn't yet ready for that kind of
headache.

Richard

Re: [RFD] MIPS/gas: Optimisation cannot be set to 0

[Maciej W. Rozycki]

On Sun, 11 Nov 2007, Richard Sandiford wrote:

> Right.  There's not really any point doing it for MIPS16 code.  The only
> kind of reordering you get is when GAS fills a delay slot that GCC didn't
> know how to fill, so ".set noreorder" would mean "don't make this
> function as small as you can".  That'd be a bit perverse on a target
> where size is really the only thing that matters.  (In contrast,
> GCC might have had good reasons to do what it did for non-MIPS16 code,
> especially on targets like the VR413x.)

 Obviously ".set noreorder" would make sense here only if GCC had 
scheduled the delay slot itself already.

> It would be quite hard for GCC to calculate precise (rather than
> conversatively-correct) lengths for every MIPS16 instruction.
> One of the problems is that the range of unextended PC-relative
> instructions depends on the low bits of the instruction's address,
> and GCC's length calculation code isn't yet ready for that kind of
> headache.

 Given the current situation it would probably be quite wise if GCC took 
the possibility for the jump to be swapped with the preceding instruction 
into account and adjusted DWARF-2 records accordingly.  Alternatively, as 
suggested by David Ung, the compact JRC/JALRC jumps could be used (where 
applicable and if permitted by the instruction set selected) keeping the 
size of code the same; though I can imagine a small performance penalty 
here resulting from one more pipeline stage being left unfilled between 
the jump is taken and its target executed.

  Maciej

Re: [RFD] MIPS/gas: Optimisation cannot be set to 0

[Richard Sandiford]

"Maciej W. Rozycki" <macro@mips.com> writes:
> On Sun, 11 Nov 2007, Richard Sandiford wrote:
>> Right.  There's not really any point doing it for MIPS16 code.  The only
>> kind of reordering you get is when GAS fills a delay slot that GCC didn't
>> know how to fill, so ".set noreorder" would mean "don't make this
>> function as small as you can".  That'd be a bit perverse on a target
>> where size is really the only thing that matters.  (In contrast,
>> GCC might have had good reasons to do what it did for non-MIPS16 code,
>> especially on targets like the VR413x.)
>
>  Obviously ".set noreorder" would make sense here only if GCC had 
> scheduled the delay slot itself already.

My point was that on targets with alignment-sensitive pipelines
like the VR413x's, GCC effectively schedules _all_ delay slots,
even if it leaves some unfilled which GAS thinks could be filled.
So it makes sense for the whole function to be ".set noreorder"
in that case.  In contrast, MIPS16 code is only ".set noreorder"
in cases where GCC has managed to do something useful with a
delay slot.

Richard

Re: [RFD] MIPS/gas: Optimisation cannot be set to 0

[Thiemo Seufer]

Ian Lance Taylor wrote:
> "Maciej W. Rozycki" <macro@mips.com> writes:
> 
> > Wed Feb 12 14:36:29 1997  Ian Lance Taylor  <ian@cygnus.com>
> > 
> > 	* config/tc-mips.c (md_parse_option): When debugging, set
> > 	mips_optimize to 1, not 0.
> 
> My vague recollection of the problem is that at least at that time gcc
> would always pass a -O option to the assembler.  When not optimizing,
> it would pass -O0.  However, inserting NOPs for MIPS variants which do
> not need them does not help debugging.
> 
> Clearly failing to provide a way to set mips_optimize to 0 was an
> oversight.
> 
> >  I have no strong preference, but I think gas should have a mode where its 
> > input is assembled intact.  And swapping branches may produce surprising 
> > interactions with debugging information.  I am therefore in favour to the 
> > first proposal above, but I would like to hear from the others.
> 
> I'm not sure I see the relevance of "assembled intact."  The source is
> assembled just as the user specified with -O1.  -O1 simply inhibits
> adding additional unnecessary nop instructions--nop instructions that
> were required for MIPS ISA I but not for later ISAs.
> 
> At -O1 branches are not swapped, so debugging is not impaired.
> 
> I agree there should be a way to set mips_optimize to 0, but we should
> ensure that gcc -O0 does not set mips_optimize to 0 when assembling.

I plan to commit the appended patch to fix this. Comments?


Thiemo


2007-11-14  Thiemo Seufer  <ths@mips.com>

	* config/tc-mips.c (md_parse_option): Match mips_optimize to the -O
	option supplied, but still keep mips_optimize == 2 as default value.

Index: head/gas/config/tc-mips.c
===================================================================
--- head.orig/gas/config/tc-mips.c	2007-11-14 17:41:58.000000000 +0000
+++ head/gas/config/tc-mips.c	2007-11-14 17:51:43.000000000 +0000
@@ -11013,7 +11013,11 @@
       break;
 
     case 'O':
-      if (arg && arg[0] == '0')
+      if (arg == NULL)
+	mips_optimize = 1;
+      else if (arg[0] == '0')
+	mips_optimize = 0;
+      else if (arg[0] == '1')
 	mips_optimize = 1;
       else
 	mips_optimize = 2;