Re: gdb-patch mailing list

Re: gdb-patch mailing list

[Michael]

Joel,

Ok, I'll have a look at it... I don't have any other thing to do anyway. 
In the mean time you could perhaps try to find out why my email doesn't 
appear on the mailing list anymore.

Michael.

Joel Brobecker wrote:
> Michael,
>
> The gdb-patches mailing list should work. Can you try using it for
> the next iteration?
>
> Again, please state the intent of the patch. What are you fixing?
> I can often guess from the patch itself, but sometimes I get it wrong.
> A good way of presenting your change, for instance, is to show the debugger
> output before and after your patch,
>
> I am realizing that perhaps you don't know how to read the contents
> of patch files? For instance:
>
>   
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index 753fbb0..bc71679 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>>     
>
> This bit of the patch says that you are modifying the ChangeLog.
> I mentioned in a previous message that it's simpler for you to send
> the entry as inlined text inside the body of your email, rather than
> as a patch, but I don't mind.  However, looking further into the patch,
> it says that you are proposing the following modifications to this file:
>
>
>   
>> -	Add new tracepoint action teval.
>> -	* tracepoint.c (teval_pseudocommand): New function.
>> -	(validate_actionline): Add teval action case.
>> -	(encode_actions): Ditto.
>> -	(_initialize_tracepoint): Define teval pseudocommand.
>> -	* NEWS: Mention teval.
>>     
> [...]
>
> The leading minus '-' signs mean that you propose to remove this entry
> (and many other entries). I don't think you really meant that.
>
> I couldn't locate any change that seemed to add an entry, but maybe
> I missed it, since the diff was so large and mostly irrelevant.
>
>   
>> -/* Print the status word STATUS.  */
>> -
>> -static void
>> -print_i387_status_word (unsigned int status, struct ui_file *file)
>> +/* print the status word */
>> +static void print_i387_status_word (unsigned int status, struct ui_file *file)
>>     
>
> This part of the patch says that:
>   (1) You are replacing the following comment:
>         /* Print the status word STATUS.  */
>       with:
>         /* print the status word */
>   (2) You are removing the newline between "static void" and
>       the function name "print_i387_status_word"
>
> I don't think that (1) was really intended, was it? Your version
> brings no additional information, but at the same time no longer
> follow the GNU coding style: Comments should be English sentences
> starting a capital letter and ending with a dot.  Not also the two
> spaces after the dot, which is mandated by the GNU Coding Style.
>
>   
>> -  fprintf_filtered (file, "Status Word:         %s",
>> -		    hex_string_custom (status, 4));
>> +  fprintf_filtered (file, "status word              : %s\n", hex_string_custom(status, 4));
>>     
>
> You made a formatting change which is incorrect: The code should go no
> further than column 78-80.  Please leave the call to hex_string_custom
> on the next line where it was.  Make sure, when you send a new patch,
> that the diff does not show any difference on this line, particularly
> differences in spaces.
>
>   
>> +  fprintf_filtered (file, "%s ", (status & 0x0020) ? "PE" : "  "); /* precision */
>>     
>
> I think that these added comments are superfluous - any developer working
> on that code should have a basic knowledge of this FPU and thus know
> what these abbreviations mean.  I do not mind them so much, except that
> they cause the line to exceed 80 chars, or be sufficient close to it,
> that these comments should be on a line of their own.
>
>   

Re: gdb-patch mailing list

[Michael]

Michael wrote:
> Joel,
>
> Ok, I'll have a look at it... I don't have any other thing to do 
> anyway. In the mean time you could perhaps try to find out why my 
> email doesn't appear on the mailing list anymore.
>
> Michael.
>
> Joel Brobecker wrote:
>> Michael,
>>
>> The gdb-patches mailing list should work. Can you try using it for
>> the next iteration?
>>
>> Again, please state the intent of the patch. What are you fixing?
>> I can often guess from the patch itself, but sometimes I get it wrong.
>> A good way of presenting your change, for instance, is to show the 
>> debugger
>> output before and after your patch,
>>
>> I am realizing that perhaps you don't know how to read the contents
>> of patch files? For instance:
>>
>>  
>>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>>> index 753fbb0..bc71679 100644
>>> --- a/gdb/ChangeLog
>>> +++ b/gdb/ChangeLog
>>>     
>>
>> This bit of the patch says that you are modifying the ChangeLog.
>> I mentioned in a previous message that it's simpler for you to send
>> the entry as inlined text inside the body of your email, rather than
>> as a patch, but I don't mind.  However, looking further into the patch,
>> it says that you are proposing the following modifications to this file:
>>
>>
>>  
>>> -    Add new tracepoint action teval.
>>> -    * tracepoint.c (teval_pseudocommand): New function.
>>> -    (validate_actionline): Add teval action case.
>>> -    (encode_actions): Ditto.
>>> -    (_initialize_tracepoint): Define teval pseudocommand.
>>> -    * NEWS: Mention teval.
>>>     
>> [...]
>>
>> The leading minus '-' signs mean that you propose to remove this entry
>> (and many other entries). I don't think you really meant that.
this is not mine, I think you are in error??
>>
>> I couldn't locate any change that seemed to add an entry, but maybe
>> I missed it, since the diff was so large and mostly irrelevant.
>>
>>  
>>> -/* Print the status word STATUS.  */
>>> -
>>> -static void
>>> -print_i387_status_word (unsigned int status, struct ui_file *file)
>>> +/* print the status word */
>>> +static void print_i387_status_word (unsigned int status, struct 
>>> ui_file *file)
>>>     
>>
>> This part of the patch says that:
>>   (1) You are replacing the following comment:
>>         /* Print the status word STATUS.  */
>>       with:
>>         /* print the status word */
>>   (2) You are removing the newline between "static void" and
>>       the function name "print_i387_status_word"
>>
>> I don't think that (1) was really intended, was it? Your version
>> brings no additional information, but at the same time no longer
>> follow the GNU coding style: Comments should be English sentences
>> starting a capital letter and ending with a dot.  Not also the two
>> spaces after the dot, which is mandated by the GNU Coding Style.
>>
>>  
>>> -  fprintf_filtered (file, "Status Word:         %s",
>>> -            hex_string_custom (status, 4));
>>> +  fprintf_filtered (file, "status word              : %s\n", 
>>> hex_string_custom(status, 4));
>>>     
>>
>> You made a formatting change which is incorrect: The code should go no
>> further than column 78-80.  Please leave the call to hex_string_custom
>> on the next line where it was.  Make sure, when you send a new patch,
>> that the diff does not show any difference on this line, particularly
>> differences in spaces.
>>
>>  
>>> +  fprintf_filtered (file, "%s ", (status & 0x0020) ? "PE" : "  "); 
>>> /* precision */
>>>     
>>
>> I think that these added comments are superfluous - any developer 
>> working
>> on that code should have a basic knowledge of this FPU and thus know
>> what these abbreviations mean.  I do not mind them so much, except that
>> they cause the line to exceed 80 chars, or be sufficient close to it,
>> that these comments should be on a line of their own.
>>
>>   
>
>

Re: gdb-patch mailing list

[Joel Brobecker]

[I removed gdb-patches by accident - resending]

> >>>-    Add new tracepoint action teval.
> >>>-    * tracepoint.c (teval_pseudocommand): New function.
> >>>-    (validate_actionline): Add teval action case.
> >>>-    (encode_actions): Ditto.
> >>>-    (_initialize_tracepoint): Define teval pseudocommand.
> >>>-    * NEWS: Mention teval.
> >>[...]
> >>
> >>The leading minus '-' signs mean that you propose to remove this entry
> >>(and many other entries). I don't think you really meant that.
> this is not mine, I think you are in error??

This is exactly what I am saying: The patch you sent, if applied,
would cause all these entries in the ChangeLog file to be removed
(once approved, we apply use "patch" to apply the patch automatically,
we do not re-do the changes manually).

We both agree that this is not intentional. This is why you have
to double-check the contents of your patch before you send it,
to make sure that it contains all the changes that are you are
proposing to make, but also *only* the changes that you are proposing
to make.

It will probably simplify your life to just start over.  Checkout
the sources from scratch, make your changes manually, and then
extract the diff.  If you used CVS, then:
    % cvs diff -up FILE1 FILE2 FILE3 [etc...] > patch.diff
If you used git:
    % git diff > patch.diff

But regardless, always make sure to double-check your patch before
sending it.  You may have slipped some unintentional changes by
accident, you never know.

-- 
Joel

Re: gdb-patch mailing list

[Michael]

[-- Attachment #1: Type: text/plain, Size: 788 bytes --]

Joel Brobecker wrote:
> Michael,
>
>   
>> -2009-12-30  Stan Shebs  <stan@codesourcery.com>
>> -
>> -       * tracepoint.c (trace_status_command): Add some status output.
>> -
>>
>> on the status word output, maybe he knows how to fix my diff?!
>>     
>
> Is there anyone around you that has a bit of experience dealing with
> version-control systems, diff, patch, etc? I can help you, but doing so
> by email is taking a very large amount of time for both of us. I think
> that a verbal communication might help speed things along. Otherwise,
> first things first, get a fresh checkout of the GDB sources, do not
> touch the sources, just build GDB, and then get back to me.
>
>   
I suppose that indeed something went wrong, don't know what exactly, 
here's the new patch :)

Michael.

[-- Attachment #2: gdb.diff --]
[-- Type: text/x-patch, Size: 6383 bytes --]

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 188347f..bc3337b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2010-01-02  Michael Baars  <development@codenamezero.org>
+
+	* i387-tdep.c: Maintenance update on 'info float'
+
 2010-01-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* cli/cli-script.c (process_next_line): Rename p1 as p_end and p2 as
diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index 3fb5b56..e5766d7 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -113,86 +113,106 @@ print_i387_ext (struct gdbarch *gdbarch,
     fputs_filtered (" Unsupported", file);
 }
 
-/* Print the status word STATUS.  */
-
+/* Print the status word. */
 static void
 print_i387_status_word (unsigned int status, struct ui_file *file)
 {
-  fprintf_filtered (file, "Status Word:         %s",
-		    hex_string_custom (status, 4));
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0001) ? "IE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0002) ? "DE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0004) ? "ZE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0008) ? "OE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0010) ? "UE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0020) ? "PE" : "  ");
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0080) ? "ES" : "  ");
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0040) ? "SF" : "  ");
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0100) ? "C0" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0200) ? "C1" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0400) ? "C2" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x4000) ? "C3" : "  ");
-
-  fputs_filtered ("\n", file);
-
-  fprintf_filtered (file,
-		    "                       TOP: %d\n", ((status >> 11) & 7));
+  fprintf_filtered (file, "status word              : %s\n",
+    hex_string_custom(status, 4));
+
+  fprintf_filtered (file, "  exception flags        : ");
+
+  /* Precision */
+  fprintf_filtered (file, "%s ", (status & 0x0020) ? "PE" : "  ");
+  /* Underflow */
+  fprintf_filtered (file, "%s ", (status & 0x0010) ? "UE" : "  ");
+  /* Overflow */
+  fprintf_filtered (file, "%s ", (status & 0x0008) ? "OE" : "  ");
+  /* Zero Devide */
+  fprintf_filtered (file, "%s ", (status & 0x0004) ? "ZE" : "  ");
+  /* Denormalized operand */
+  fprintf_filtered (file, "%s ", (status & 0x0002) ? "DE" : "  ");
+  /* Invalid operation */
+  fprintf_filtered (file, "%s ", (status & 0x0001) ? "IE" : "  ");
+
+  fprintf_filtered (file, "\n");
+
+  fprintf_filtered (file, "  stack fault            : %s\n",
+    (status & 0x0040) ? "SF" : "  ");
+  fprintf_filtered (file, "  error summary status   : %s\n",
+    (status & 0x0080) ? "ES" : "  ");
+
+  fprintf_filtered (file, "  condition code         : ");
+
+  fprintf_filtered (file, "%s ", (status & 0x4000) ? "C3" : "  ");
+  fprintf_filtered (file, "%s ", (status & 0x0400) ? "C2" : "  ");
+  fprintf_filtered (file, "%s ", (status & 0x0200) ? "C1" : "  ");
+  fprintf_filtered (file, "%s ", (status & 0x0100) ? "C0" : "  ");
+  fprintf_filtered (file, "\n");
+
+  fprintf_filtered (file, "  top of stack (TOP)     : %d\n",
+    ((status >> 11) & 7));
 }
 
-/* Print the control word CONTROL.  */
-
+/* Print the control word. */
 static void
 print_i387_control_word (unsigned int control, struct ui_file *file)
 {
-  fprintf_filtered (file, "Control Word:        %s",
-		    hex_string_custom (control, 4));
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (control & 0x0001) ? "IM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0002) ? "DM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0004) ? "ZM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0008) ? "OM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0010) ? "UM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0020) ? "PM" : "  ");
+  fprintf_filtered(file, "control word             : %s\n",
+    hex_string_custom(control, 4));
 
-  fputs_filtered ("\n", file);
+  fprintf_filtered(file, "  exception masks        : ");
+
+  /* Precision */
+  fprintf_filtered (file, "%s ", (control & 0x0020) ? "PM" : "  ");
+  /* Underflow */
+  fprintf_filtered (file, "%s ", (control & 0x0010) ? "UM" : "  ");
+  /* Overflow */
+  fprintf_filtered (file, "%s ", (control & 0x0008) ? "OM" : "  ");
+  /* Zero devide */
+  fprintf_filtered (file, "%s ", (control & 0x0004) ? "ZM" : "  ");
+  /* Denormalized operand */
+  fprintf_filtered (file, "%s ", (control & 0x0002) ? "DM" : "  ");
+  /* Invalid operation */
+  fprintf_filtered (file, "%s ", (control & 0x0001) ? "IM" : "  ");
+
+  fprintf_filtered (file, "\n");
+
+  fprintf_filtered (file, "  precision control (PC) : ");
 
-  fputs_filtered ("                       PC: ", file);
   switch ((control >> 8) & 3)
-    {
+  {
     case 0:
-      fputs_filtered ("Single Precision (24-bits)\n", file);
+      fprintf_filtered (file, "24-bits (single precision)\n");
       break;
     case 1:
-      fputs_filtered ("Reserved\n", file);
+      fprintf_filtered (file, "(reserved)\n");
       break;
     case 2:
-      fputs_filtered ("Double Precision (53-bits)\n", file);
+      fprintf_filtered (file, "53-bits (double precision)\n");
       break;
     case 3:
-      fputs_filtered ("Extended Precision (64-bits)\n", file);
+      fprintf_filtered (file, "64-bits (extended precision)\n");
       break;
-    }
-      
-  fputs_filtered ("                       RC: ", file);
+  }
+
+  fprintf_filtered (file, "  rounding control (RC)  : ");
+
   switch ((control >> 10) & 3)
-    {
+  {
     case 0:
-      fputs_filtered ("Round to nearest\n", file);
+      fprintf_filtered (file, "round to nearest or even\n");
       break;
     case 1:
-      fputs_filtered ("Round down\n", file);
+      fprintf_filtered (file, "round down (towards -inf)\n");
       break;
     case 2:
-      fputs_filtered ("Round up\n", file);
+      fprintf_filtered (file, "round up (toward +inf)\n");
       break;
     case 3:
-      fputs_filtered ("Round toward zero\n", file);
+      fprintf_filtered (file, "chop (truncate toward zero)\n");
       break;
-    }
+  }
 }
 
 /* Print out the i387 floating point state.  Note that we ignore FRAME

Re: [RFA/i387] improve the output of 'info float' (was: "Re: gdb-patch mailing list")

[Joel Brobecker]

Michael,

I changed the subject to something meaningful; you have a *much* higher
chance of attracting the eye of the appropriate maintainer if you use
descriptive subjects.

> I suppose that indeed something went wrong, don't know what exactly,
> here's the new patch :)

Much better indeed! Glad to see that things are getting in order.

I would have liked to have a more detailed description of *what* you
are trying to achieve, here, and also *why*. The changes you are proposing
on some of the code are very small and the size of the blocks you are
editing is large enough to make spotting differences harder. <<Maintenance
update on 'info float'>> is just not informative at all, and ...

> +2010-01-02  Michael Baars  <development@codenamezero.org>
> +
> +	* i387-tdep.c: Maintenance update on 'info float'

... not acceptable as a ChangeLog entry.

  1. You need to specify which functions are impacted;
  2. You need to provide a more descriptive text. For instance:
     Improve the formatting of the "info float" command, or some such.
     Even that, considering your change, seems too vague.

Have a look at gdb/ChangeLog-2009 to get a feel of how others have
written their ChangeLog entries.

The following is only going to be a unofficial review, because I will
defer to the judgment of our x86/x86_64 maintainer. But he's fairly
busy, so I will help you cleanup the patch before-hand. MarkK, feel
free to take over at any time!

> -/* Print the status word STATUS.  */
> -
> +/* Print the status word. */

This part, I am afraid, is not acceptable.  You version of the description
is less informative than the initial one.  FYI, there is a convention
at least in GDB to spell in the function documentation the name of
the parameters in all caps... So, to a GDB developer, the initial
function description makes perfect sense.

> +  fprintf_filtered (file, "status word              : %s\n",
> +    hex_string_custom(status, 4));

The formatting is incorrect. hex_string_custom should be indented
to match the indentation one column right of the '(' above:

  fprintf_filtered (file, "status word              : %s\n",
                    hex_string_custom(status, 4));

(make sure to use tabs where you would otherwise use 8 spaces)

> +  /* Precision */
> +  fprintf_filtered (file, "%s ", (status & 0x0020) ? "PE" : "  ");
> +  /* Underflow */
> +  fprintf_filtered (file, "%s ", (status & 0x0010) ? "UE" : "  ");
> +  /* Overflow */
> +  fprintf_filtered (file, "%s ", (status & 0x0008) ? "OE" : "  ");
> +  /* Zero Devide */
> +  fprintf_filtered (file, "%s ", (status & 0x0004) ? "ZE" : "  ");
> +  /* Denormalized operand */
> +  fprintf_filtered (file, "%s ", (status & 0x0002) ? "DE" : "  ");
> +  /* Invalid operation */
> +  fprintf_filtered (file, "%s ", (status & 0x0001) ? "IE" : "  ");

I personally think that the added comments are overkill, but I'm
otherwise OK with them. Mark might have a different opinion on this.

I have a question, though, why did you reverse the order of the flags
being printed? I supect because of endianness?

> +  fprintf_filtered (file, "  stack fault            : %s\n",
> +    (status & 0x0040) ? "SF" : "  ");
> +  fprintf_filtered (file, "  error summary status   : %s\n",
> +    (status & 0x0080) ? "ES" : "  ");
> +
> +  fprintf_filtered (file, "  condition code         : ");
> +
> +  fprintf_filtered (file, "%s ", (status & 0x4000) ? "C3" : "  ");
> +  fprintf_filtered (file, "%s ", (status & 0x0400) ? "C2" : "  ");
> +  fprintf_filtered (file, "%s ", (status & 0x0200) ? "C1" : "  ");
> +  fprintf_filtered (file, "%s ", (status & 0x0100) ? "C0" : "  ");
> +  fprintf_filtered (file, "\n");

Personally, I liked the original output format better. Mark?

> -/* Print the control word CONTROL.  */
> -
> +/* Print the control word. */

Same as above - let's just keep the original function description.

>  static void
>  print_i387_control_word (unsigned int control, struct ui_file *file)

The general comments I made above also apply to this function.

I will let Mark tell you whether he likes your change of output or not.
Personally, I liked the previous output better - much more compact
and easier to read, there are too many decorations in your proposed
output and it's harder to isolate the relevant information.

This reminds me of the studies made by Airbus and Boeing about
intrument readability. Boeing chose to keep the needle approach
in various indicators because pilots can tell at a glance that
a needle is roughly at its intended position, whereas it's impossible
to do when the indicator is a plain number. You have to read the number
and interpret it before you can tell.

-- 
Joel

Re: [RFA/i387] improve the output of 'info float' (was: "Re: gdb-patch mailing list")

[Mark Kettenis]

> Date: Sun, 3 Jan 2010 09:32:13 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> 
> Michael,
> 
> I changed the subject to something meaningful; you have a *much* higher
> chance of attracting the eye of the appropriate maintainer if you use
> descriptive subjects.

Yup.  I didn't notice it.  So either I didn't receive the origional
mail or I deleted it immediately based on the subject.  Can you send
the diff again?

> I will let Mark tell you whether he likes your change of output or not.
> Personally, I liked the previous output better - much more compact
> and easier to read, there are too many decorations in your proposed
> output and it's harder to isolate the relevant information.

If you send your diff again, can you include an example of how the
output looks?  I must warn you that I'm likely to agree with Joel here
though.

Re: [RFA/i387] improve the output of 'info float' (was: "Re: gdb-patch mailing list")

[Eli Zaretskii]

> Date: Sun, 3 Jan 2010 09:32:13 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > +  /* Precision */
> > +  fprintf_filtered (file, "%s ", (status & 0x0020) ? "PE" : "  ");
> > +  /* Underflow */
> > +  fprintf_filtered (file, "%s ", (status & 0x0010) ? "UE" : "  ");
> > +  /* Overflow */
> > +  fprintf_filtered (file, "%s ", (status & 0x0008) ? "OE" : "  ");
> > +  /* Zero Devide */
> > +  fprintf_filtered (file, "%s ", (status & 0x0004) ? "ZE" : "  ");
> > +  /* Denormalized operand */
> > +  fprintf_filtered (file, "%s ", (status & 0x0002) ? "DE" : "  ");
> > +  /* Invalid operation */
> > +  fprintf_filtered (file, "%s ", (status & 0x0001) ? "IE" : "  ");
> 
> I personally think that the added comments are overkill, but I'm
> otherwise OK with them. Mark might have a different opinion on this.

I'm not Mark, but how exactly are these comments overkill?  Unless
someone knows by heart the bits in the x87 FP Status word, or can
decipher the two-letter abbreviations without consulting the Intel
manuals, I'd say they add significantly to the readability of the
source code.

However, these comments need to be formatted according to coding
standards, as in

  /* Denormalized operand.  */

(i.e. ended with a period and 2 spaces), and also proofread for typos,
like this one:

> > +  /* Zero Devide */

Re: [RFA/i387] improve the output of 'info float'

[Michael]

[-- Attachment #1: Type: text/plain, Size: 1921 bytes --]

Hi all,

Hope this does a better job then.

Mark Kettenis wrote:
>> Date: Sun, 3 Jan 2010 09:32:13 +0400
>> From: Joel Brobecker <brobecker@adacore.com>
>>
>> Michael,
>>
>> I changed the subject to something meaningful; you have a *much* higher
>> chance of attracting the eye of the appropriate maintainer if you use
>> descriptive subjects.
>>     
>
> Yup.  I didn't notice it.  So either I didn't receive the origional
> mail or I deleted it immediately based on the subject.  Can you send
> the diff again?
>
>   
>> I will let Mark tell you whether he likes your change of output or not.
>> Personally, I liked the previous output better - much more compact
>> and easier to read, there are too many decorations in your proposed
>> output and it's harder to isolate the relevant information.
>>     
>
> If you send your diff again, can you include an example of how the
> output looks?  I must warn you that I'm likely to agree with Joel here
> though.
>
>   
New output looks like:

Temporary breakpoint 1, main () at ./main.c:6
6        double x = rand();
(gdb) info float
  R7: Empty   0x00000000000000000000
  R6: Empty   0x00000000000000000000
  R5: Empty   0x00000000000000000000
  R4: Empty   0x00000000000000000000
  R3: Empty   0x00000000000000000000
  R2: Empty   0x00000000000000000000
  R1: Empty   0x00000000000000000000
=>R0: Empty   0x00000000000000000000

status word              : 0x0000
  exception flags        :                  
  stack fault            :  
  error summary status   :  
  condition code         :            
  top of stack (TOP)     : 0
control word             : 0x037f
  exception masks        : PM UM OM ZM DM IM
  precision control (PC) : 64-bits (extended precision)
  rounding control (RC)  : round to nearest or even
Tag Word:            0xffff
Instruction Pointer: 0x00:0x00000000
Operand Pointer:     0x00:0x00000000
Opcode:              0x0000


Regards,
Michael

[-- Attachment #2: gdb.diff --]
[-- Type: text/x-patch, Size: 6765 bytes --]

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 188347f..d747e79 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@
+2010-01-02  Michael Baars  <development@codenamezero.org>
+
+	* i387-tdep.c:
+	    (print_i387_status_word): Adapted the output such that the least 
+	      significant bits appear at the right and added a more 
+	      comprehensive description of the flags involved.
+	    (print_i387_control_word): Adapted the output such that the least
+	      significant bits appear at the right.
+	    
+	    naming conventions are conform the intel i387 dx programmer's 
+	    reference manual (1989)
+
 2010-01-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* cli/cli-script.c (process_next_line): Rename p1 as p_end and p2 as
diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index 3fb5b56..e5766d7 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -113,86 +113,106 @@ print_i387_ext (struct gdbarch *gdbarch,
     fputs_filtered (" Unsupported", file);
 }
 
-/* Print the status word STATUS.  */
-
+/* Print the status word. */
 static void
 print_i387_status_word (unsigned int status, struct ui_file *file)
 {
-  fprintf_filtered (file, "Status Word:         %s",
-		    hex_string_custom (status, 4));
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0001) ? "IE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0002) ? "DE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0004) ? "ZE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0008) ? "OE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0010) ? "UE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0020) ? "PE" : "  ");
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0080) ? "ES" : "  ");
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0040) ? "SF" : "  ");
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0100) ? "C0" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0200) ? "C1" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0400) ? "C2" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x4000) ? "C3" : "  ");
-
-  fputs_filtered ("\n", file);
-
-  fprintf_filtered (file,
-		    "                       TOP: %d\n", ((status >> 11) & 7));
+  fprintf_filtered (file, "status word              : %s\n",
+    hex_string_custom(status, 4));
+
+  fprintf_filtered (file, "  exception flags        : ");
+
+  /* Precision */
+  fprintf_filtered (file, "%s ", (status & 0x0020) ? "PE" : "  ");
+  /* Underflow */
+  fprintf_filtered (file, "%s ", (status & 0x0010) ? "UE" : "  ");
+  /* Overflow */
+  fprintf_filtered (file, "%s ", (status & 0x0008) ? "OE" : "  ");
+  /* Zero Devide */
+  fprintf_filtered (file, "%s ", (status & 0x0004) ? "ZE" : "  ");
+  /* Denormalized operand */
+  fprintf_filtered (file, "%s ", (status & 0x0002) ? "DE" : "  ");
+  /* Invalid operation */
+  fprintf_filtered (file, "%s ", (status & 0x0001) ? "IE" : "  ");
+
+  fprintf_filtered (file, "\n");
+
+  fprintf_filtered (file, "  stack fault            : %s\n",
+    (status & 0x0040) ? "SF" : "  ");
+  fprintf_filtered (file, "  error summary status   : %s\n",
+    (status & 0x0080) ? "ES" : "  ");
+
+  fprintf_filtered (file, "  condition code         : ");
+
+  fprintf_filtered (file, "%s ", (status & 0x4000) ? "C3" : "  ");
+  fprintf_filtered (file, "%s ", (status & 0x0400) ? "C2" : "  ");
+  fprintf_filtered (file, "%s ", (status & 0x0200) ? "C1" : "  ");
+  fprintf_filtered (file, "%s ", (status & 0x0100) ? "C0" : "  ");
+  fprintf_filtered (file, "\n");
+
+  fprintf_filtered (file, "  top of stack (TOP)     : %d\n",
+    ((status >> 11) & 7));
 }
 
-/* Print the control word CONTROL.  */
-
+/* Print the control word. */
 static void
 print_i387_control_word (unsigned int control, struct ui_file *file)
 {
-  fprintf_filtered (file, "Control Word:        %s",
-		    hex_string_custom (control, 4));
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (control & 0x0001) ? "IM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0002) ? "DM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0004) ? "ZM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0008) ? "OM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0010) ? "UM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0020) ? "PM" : "  ");
+  fprintf_filtered(file, "control word             : %s\n",
+    hex_string_custom(control, 4));
 
-  fputs_filtered ("\n", file);
+  fprintf_filtered(file, "  exception masks        : ");
+
+  /* Precision */
+  fprintf_filtered (file, "%s ", (control & 0x0020) ? "PM" : "  ");
+  /* Underflow */
+  fprintf_filtered (file, "%s ", (control & 0x0010) ? "UM" : "  ");
+  /* Overflow */
+  fprintf_filtered (file, "%s ", (control & 0x0008) ? "OM" : "  ");
+  /* Zero devide */
+  fprintf_filtered (file, "%s ", (control & 0x0004) ? "ZM" : "  ");
+  /* Denormalized operand */
+  fprintf_filtered (file, "%s ", (control & 0x0002) ? "DM" : "  ");
+  /* Invalid operation */
+  fprintf_filtered (file, "%s ", (control & 0x0001) ? "IM" : "  ");
+
+  fprintf_filtered (file, "\n");
+
+  fprintf_filtered (file, "  precision control (PC) : ");
 
-  fputs_filtered ("                       PC: ", file);
   switch ((control >> 8) & 3)
-    {
+  {
     case 0:
-      fputs_filtered ("Single Precision (24-bits)\n", file);
+      fprintf_filtered (file, "24-bits (single precision)\n");
       break;
     case 1:
-      fputs_filtered ("Reserved\n", file);
+      fprintf_filtered (file, "(reserved)\n");
       break;
     case 2:
-      fputs_filtered ("Double Precision (53-bits)\n", file);
+      fprintf_filtered (file, "53-bits (double precision)\n");
       break;
     case 3:
-      fputs_filtered ("Extended Precision (64-bits)\n", file);
+      fprintf_filtered (file, "64-bits (extended precision)\n");
       break;
-    }
-      
-  fputs_filtered ("                       RC: ", file);
+  }
+
+  fprintf_filtered (file, "  rounding control (RC)  : ");
+
   switch ((control >> 10) & 3)
-    {
+  {
     case 0:
-      fputs_filtered ("Round to nearest\n", file);
+      fprintf_filtered (file, "round to nearest or even\n");
       break;
     case 1:
-      fputs_filtered ("Round down\n", file);
+      fprintf_filtered (file, "round down (towards -inf)\n");
       break;
     case 2:
-      fputs_filtered ("Round up\n", file);
+      fprintf_filtered (file, "round up (toward +inf)\n");
       break;
     case 3:
-      fputs_filtered ("Round toward zero\n", file);
+      fprintf_filtered (file, "chop (truncate toward zero)\n");
       break;
-    }
+  }
 }
 
 /* Print out the i387 floating point state.  Note that we ignore FRAME

Re: [RFA/i387] improve the output of 'info float'

[Michael]

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]

Hi all,

Hope this does a better job then. I also removed the typo's from the 
comments.

Mark Kettenis wrote:
>> Date: Sun, 3 Jan 2010 09:32:13 +0400
>> From: Joel Brobecker <brobecker@adacore.com>
>>
>> Michael,
>>
>> I changed the subject to something meaningful; you have a *much* higher
>> chance of attracting the eye of the appropriate maintainer if you use
>> descriptive subjects.
>>     
>
> Yup.  I didn't notice it.  So either I didn't receive the origional
> mail or I deleted it immediately based on the subject.  Can you send
> the diff again?
>
>   
>> I will let Mark tell you whether he likes your change of output or not.
>> Personally, I liked the previous output better - much more compact
>> and easier to read, there are too many decorations in your proposed
>> output and it's harder to isolate the relevant information.
>>     
>
> If you send your diff again, can you include an example of how the
> output looks?  I must warn you that I'm likely to agree with Joel here
> though.
>
>   
New output looks like:

Does not display correctly in the previous email, you should check it 
for yourself I suppose.

Regards,
Michael


[-- Attachment #2: gdb.diff --]
[-- Type: text/x-patch, Size: 6766 bytes --]

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 188347f..d747e79 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@
+2010-01-02  Michael Baars  <development@codenamezero.org>
+
+	* i387-tdep.c:
+	    (print_i387_status_word): Adapted the output such that the least 
+	      significant bits appear at the right and added a more 
+	      comprehensive description of the flags involved.
+	    (print_i387_control_word): Adapted the output such that the least
+	      significant bits appear at the right.
+	    
+	    naming conventions are conform the intel i387 dx programmer's 
+	    reference manual (1989)
+
 2010-01-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* cli/cli-script.c (process_next_line): Rename p1 as p_end and p2 as
diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index 3fb5b56..e5766d7 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -113,86 +113,106 @@ print_i387_ext (struct gdbarch *gdbarch,
     fputs_filtered (" Unsupported", file);
 }
 
-/* Print the status word STATUS.  */
-
+/* Print the status word. */
 static void
 print_i387_status_word (unsigned int status, struct ui_file *file)
 {
-  fprintf_filtered (file, "Status Word:         %s",
-		    hex_string_custom (status, 4));
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0001) ? "IE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0002) ? "DE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0004) ? "ZE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0008) ? "OE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0010) ? "UE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0020) ? "PE" : "  ");
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0080) ? "ES" : "  ");
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0040) ? "SF" : "  ");
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0100) ? "C0" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0200) ? "C1" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0400) ? "C2" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x4000) ? "C3" : "  ");
-
-  fputs_filtered ("\n", file);
-
-  fprintf_filtered (file,
-		    "                       TOP: %d\n", ((status >> 11) & 7));
+  fprintf_filtered (file, "status word              : %s\n",
+    hex_string_custom(status, 4));
+
+  fprintf_filtered (file, "  exception flags        : ");
+
+  /* Precision */
+  fprintf_filtered (file, "%s ", (status & 0x0020) ? "PE" : "  ");
+  /* Underflow */
+  fprintf_filtered (file, "%s ", (status & 0x0010) ? "UE" : "  ");
+  /* Overflow */
+  fprintf_filtered (file, "%s ", (status & 0x0008) ? "OE" : "  ");
+  /* Zero Devide */
+  fprintf_filtered (file, "%s ", (status & 0x0004) ? "ZE" : "  ");
+  /* Denormalized operand */
+  fprintf_filtered (file, "%s ", (status & 0x0002) ? "DE" : "  ");
+  /* Invalid operation */
+  fprintf_filtered (file, "%s ", (status & 0x0001) ? "IE" : "  ");
+
+  fprintf_filtered (file, "\n");
+
+  fprintf_filtered (file, "  stack fault            : %s\n",
+    (status & 0x0040) ? "SF" : "  ");
+  fprintf_filtered (file, "  error summary status   : %s\n",
+    (status & 0x0080) ? "ES" : "  ");
+
+  fprintf_filtered (file, "  condition code         : ");
+
+  fprintf_filtered (file, "%s ", (status & 0x4000) ? "C3" : "  ");
+  fprintf_filtered (file, "%s ", (status & 0x0400) ? "C2" : "  ");
+  fprintf_filtered (file, "%s ", (status & 0x0200) ? "C1" : "  ");
+  fprintf_filtered (file, "%s ", (status & 0x0100) ? "C0" : "  ");
+  fprintf_filtered (file, "\n");
+
+  fprintf_filtered (file, "  top of stack (TOP)     : %d\n",
+    ((status >> 11) & 7));
 }
 
-/* Print the control word CONTROL.  */
-
+/* Print the control word. */
 static void
 print_i387_control_word (unsigned int control, struct ui_file *file)
 {
-  fprintf_filtered (file, "Control Word:        %s",
-		    hex_string_custom (control, 4));
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (control & 0x0001) ? "IM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0002) ? "DM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0004) ? "ZM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0008) ? "OM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0010) ? "UM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0020) ? "PM" : "  ");
+  fprintf_filtered(file, "control word             : %s\n",
+    hex_string_custom(control, 4));
 
-  fputs_filtered ("\n", file);
+  fprintf_filtered(file, "  exception masks        : ");
+
+  /* Precision */
+  fprintf_filtered (file, "%s ", (control & 0x0020) ? "PM" : "  ");
+  /* Underflow */
+  fprintf_filtered (file, "%s ", (control & 0x0010) ? "UM" : "  ");
+  /* Overflow */
+  fprintf_filtered (file, "%s ", (control & 0x0008) ? "OM" : "  ");
+  /* Zero devide */
+  fprintf_filtered (file, "%s ", (control & 0x0004) ? "ZM" : "  ");
+  /* Denormalized operand */
+  fprintf_filtered (file, "%s ", (control & 0x0002) ? "DM" : "  ");
+  /* Invalid operation */
+  fprintf_filtered (file, "%s ", (control & 0x0001) ? "IM" : "  ");
+
+  fprintf_filtered (file, "\n");
+
+  fprintf_filtered (file, "  precision control (PC) : ");
 
-  fputs_filtered ("                       PC: ", file);
   switch ((control >> 8) & 3)
-    {
+  {
     case 0:
-      fputs_filtered ("Single Precision (24-bits)\n", file);
+      fprintf_filtered (file, "24-bits (single precision)\n");
       break;
     case 1:
-      fputs_filtered ("Reserved\n", file);
+      fprintf_filtered (file, "(reserved)\n");
       break;
     case 2:
-      fputs_filtered ("Double Precision (53-bits)\n", file);
+      fprintf_filtered (file, "53-bits (double precision)\n");
       break;
     case 3:
-      fputs_filtered ("Extended Precision (64-bits)\n", file);
+      fprintf_filtered (file, "64-bits (extended precision)\n");
       break;
-    }
-      
-  fputs_filtered ("                       RC: ", file);
+  }
+
+  fprintf_filtered (file, "  rounding control (RC)  : ");
+
   switch ((control >> 10) & 3)
-    {
+  {
     case 0:
-      fputs_filtered ("Round to nearest\n", file);
+      fprintf_filtered (file, "round to nearest or even\n");
       break;
     case 1:
-      fputs_filtered ("Round down\n", file);
+      fprintf_filtered (file, "round down (towards -inf)\n");
       break;
     case 2:
-      fputs_filtered ("Round up\n", file);
+      fprintf_filtered (file, "round up (toward +inf)\n");
       break;
     case 3:
-      fputs_filtered ("Round toward zero\n", file);
+      fprintf_filtered (file, "chop (truncate toward zero)\n");
       break;
-    }
+  }
 }
 
 /* Print out the i387 floating point state.  Note that we ignore FRAME

Re: [RFA/i387] improve the output of 'info float'

[Michael]

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]

Hi all,

However, I personally still favor this version.

Regards,
Michael

Mark Kettenis wrote:
>> Date: Sun, 3 Jan 2010 09:32:13 +0400
>> From: Joel Brobecker <brobecker@adacore.com>
>>
>> Michael,
>>
>> I changed the subject to something meaningful; you have a *much* higher
>> chance of attracting the eye of the appropriate maintainer if you use
>> descriptive subjects.
>>     
>
> Yup.  I didn't notice it.  So either I didn't receive the origional
> mail or I deleted it immediately based on the subject.  Can you send
> the diff again?
>
>   
>> I will let Mark tell you whether he likes your change of output or not.
>> Personally, I liked the previous output better - much more compact
>> and easier to read, there are too many decorations in your proposed
>> output and it's harder to isolate the relevant information.
>>     
>
> If you send your diff again, can you include an example of how the
> output looks?  I must warn you that I'm likely to agree with Joel here
> though.
>
>   


[-- Attachment #2: gdb2.diff --]
[-- Type: text/x-patch, Size: 7115 bytes --]

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 188347f..d747e79 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@
+2010-01-02  Michael Baars  <development@codenamezero.org>
+
+	* i387-tdep.c:
+	    (print_i387_status_word): Adapted the output such that the least 
+	      significant bits appear at the right and added a more 
+	      comprehensive description of the flags involved.
+	    (print_i387_control_word): Adapted the output such that the least
+	      significant bits appear at the right.
+	    
+	    naming conventions are conform the intel i387 dx programmer's 
+	    reference manual (1989)
+
 2010-01-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	* cli/cli-script.c (process_next_line): Rename p1 as p_end and p2 as
diff --git a/gdb/i387-tdep.c b/gdb/i387-tdep.c
index 3fb5b56..419eb02 100644
--- a/gdb/i387-tdep.c
+++ b/gdb/i387-tdep.c
@@ -113,85 +113,86 @@ print_i387_ext (struct gdbarch *gdbarch,
     fputs_filtered (" Unsupported", file);
 }
 
-/* Print the status word STATUS.  */
-
-static void
-print_i387_status_word (unsigned int status, struct ui_file *file)
+// Print the numeric coprocessor extension (npx) status word.
+static void print_i387_status_word (unsigned int status, struct ui_file *file)
 {
-  fprintf_filtered (file, "Status Word:         %s",
-		    hex_string_custom (status, 4));
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0001) ? "IE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0002) ? "DE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0004) ? "ZE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0008) ? "OE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0010) ? "UE" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0020) ? "PE" : "  ");
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0080) ? "ES" : "  ");
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0040) ? "SF" : "  ");
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (status & 0x0100) ? "C0" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0200) ? "C1" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x0400) ? "C2" : "  ");
-  fprintf_filtered (file, " %s", (status & 0x4000) ? "C3" : "  ");
+    fprintf_filtered (file, "status word              : %s\n", hex_string_custom(status, 4));
 
-  fputs_filtered ("\n", file);
+    fprintf_filtered (file, "  exception flags        : ");
 
-  fprintf_filtered (file,
-		    "                       TOP: %d\n", ((status >> 11) & 7));
-}
+    fprintf_filtered (file, "%s ", (status & 0x0020) ? "PE" : "  ");	// precision
+    fprintf_filtered (file, "%s ", (status & 0x0010) ? "UE" : "  ");	// underflow
+    fprintf_filtered (file, "%s ", (status & 0x0008) ? "OE" : "  ");	// overflow
+    fprintf_filtered (file, "%s ", (status & 0x0004) ? "ZE" : "  ");	// zero divice
+    fprintf_filtered (file, "%s ", (status & 0x0002) ? "DE" : "  ");	// denormalized operand
+    fprintf_filtered (file, "%s ", (status & 0x0001) ? "IE" : "  ");	// invalid operation
 
-/* Print the control word CONTROL.  */
+    fprintf_filtered (file, "\n");
 
-static void
-print_i387_control_word (unsigned int control, struct ui_file *file)
+    fprintf_filtered (file, "  stack fault            : %s\n", (status & 0x0040) ? "SF" : "  ");
+    fprintf_filtered (file, "  error summary status   : %s\n", (status & 0x0080) ? "ES" : "  ");
+
+    fprintf_filtered (file, "  condition code         : ");
+
+    fprintf_filtered (file, "%s ", (status & 0x4000) ? "C3" : "  ");
+    fprintf_filtered (file, "%s ", (status & 0x0400) ? "C2" : "  ");
+    fprintf_filtered (file, "%s ", (status & 0x0200) ? "C1" : "  ");
+    fprintf_filtered (file, "%s ", (status & 0x0100) ? "C0" : "  ");
+    fprintf_filtered (file, "\n");
+
+    fprintf_filtered (file, "  top of stack (TOP)     : %d\n", ((status >> 11) & 7));
+}
+
+// Print the numeric coprocessor extension (npx) control word.
+static void print_i387_control_word (unsigned int control, struct ui_file *file)
 {
-  fprintf_filtered (file, "Control Word:        %s",
-		    hex_string_custom (control, 4));
-  fputs_filtered ("  ", file);
-  fprintf_filtered (file, " %s", (control & 0x0001) ? "IM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0002) ? "DM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0004) ? "ZM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0008) ? "OM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0010) ? "UM" : "  ");
-  fprintf_filtered (file, " %s", (control & 0x0020) ? "PM" : "  ");
+    fprintf_filtered (file, "control word             : %s\n", hex_string_custom(control, 4));
 
-  fputs_filtered ("\n", file);
+    fprintf_filtered (file, "  exception masks        : ");
+
+    fprintf_filtered (file, "%s ", (control & 0x0020) ? "PM" : "  ");   // precision
+    fprintf_filtered (file, "%s ", (control & 0x0010) ? "UM" : "  ");   // underflow
+    fprintf_filtered (file, "%s ", (control & 0x0008) ? "OM" : "  ");   // overflow
+    fprintf_filtered (file, "%s ", (control & 0x0004) ? "ZM" : "  ");   // zero divide
+    fprintf_filtered (file, "%s ", (control & 0x0002) ? "DM" : "  ");   // denormalized operand
+    fprintf_filtered (file, "%s ", (control & 0x0001) ? "IM" : "  ");   // invalid operation
+
+    fprintf_filtered (file, "\n");
 
-  fputs_filtered ("                       PC: ", file);
-  switch ((control >> 8) & 3)
+    fprintf_filtered (file, "  precision control (PC) : ");
+
+    switch ((control >> 8) & 3)
     {
-    case 0:
-      fputs_filtered ("Single Precision (24-bits)\n", file);
-      break;
-    case 1:
-      fputs_filtered ("Reserved\n", file);
-      break;
-    case 2:
-      fputs_filtered ("Double Precision (53-bits)\n", file);
-      break;
-    case 3:
-      fputs_filtered ("Extended Precision (64-bits)\n", file);
-      break;
+	case 0:
+	    fprintf_filtered (file, "24-bits (single precision)\n");
+	    break;
+	case 1:
+	    fprintf_filtered (file, "(reserved)\n");
+	    break;
+	case 2:
+	    fprintf_filtered (file, "53-bits (double precision)\n");
+	    break;
+	case 3:
+	    fprintf_filtered (file, "64-bits (extended precision)\n");
+	    break;
     }
-      
-  fputs_filtered ("                       RC: ", file);
-  switch ((control >> 10) & 3)
+
+    fprintf_filtered (file, "  rounding control (RC)  : ");
+
+    switch ((control >> 10) & 3)
     {
-    case 0:
-      fputs_filtered ("Round to nearest\n", file);
-      break;
-    case 1:
-      fputs_filtered ("Round down\n", file);
-      break;
-    case 2:
-      fputs_filtered ("Round up\n", file);
-      break;
-    case 3:
-      fputs_filtered ("Round toward zero\n", file);
-      break;
+	case 0:
+	    fprintf_filtered (file, "round to nearest or even\n");
+	    break;
+	case 1:
+	    fprintf_filtered (file, "round down (towards -inf)\n");
+	    break;
+	case 2:
+	    fprintf_filtered (file, "round up (toward +inf)\n");
+	    break;
+	case 3:
+	    fprintf_filtered (file, "chop (truncate toward zero)\n");
+	    break;
     }
 }