[2/4] x86: suppress pointless f<op>p warnings

Message ID 5A9FC38D02000078001AF430@prv-mh.provo.novell.com
State New
Headers show
Series
  • x86: FPU insn related adjustments
Related show

Commit Message

Jan Beulich March 7, 2018, 9:48 a.m.
It is pointless to warn about reversed operands when both name the same
register (granted that encoding is of little use, but anyway).

gas/
2018-03-07  Jan Beulich  <jbeulich@suse.com>

	* config/tc-i386.c (process_operands): Add accumulator check to
	"Ugh" warning check.
	* testsuite/gas/i386/fsubp.l: Drop warning expectations.

Comments

H.J. Lu March 7, 2018, 12:32 p.m. | #1
On Wed, Mar 7, 2018 at 1:48 AM, Jan Beulich <JBeulich@suse.com> wrote:
> It is pointless to warn about reversed operands when both name the same

> register (granted that encoding is of little use, but anyway).

>

> gas/

> 2018-03-07  Jan Beulich  <jbeulich@suse.com>

>

>         * config/tc-i386.c (process_operands): Add accumulator check to

>         "Ugh" warning check.

>         * testsuite/gas/i386/fsubp.l: Drop warning expectations.

>

> --- a/gas/config/tc-i386.c

> +++ b/gas/config/tc-i386.c

> @@ -6494,14 +6494,14 @@ duplicate:

>             {

>               /* Warn about some common errors, but press on regardless.

>                  The first case can be generated by gcc (<= 2.8.1).  */

> -             if (i.operands == 2)

> +             if (i.operands == 2 && !i.op[0].regs->reg_type.bitfield.acc)

>                 {

>                   /* Reversed arguments on faddp, fsubp, etc.  */

>                   as_warn (_("translating to `%s %s%s,%s%s'"), i.tm.name,

>                            register_prefix, i.op[!intel_syntax].regs->reg_name,

>                            register_prefix, i.op[intel_syntax].regs->reg_name);

>                 }

> -             else

> +             else if (i.operands == 1)

>                 {

>                   /* Extraneous `l' suffix on fp insn.  */

>                   as_warn (_("translating to `%s %s%s'"), i.tm.name,


Can we simply drop this encoding altogether?


-- 
H.J.
Jan Beulich March 7, 2018, 12:38 p.m. | #2
>>> On 07.03.18 at 13:32, <hjl.tools@gmail.com> wrote:

> On Wed, Mar 7, 2018 at 1:48 AM, Jan Beulich <JBeulich@suse.com> wrote:

>> --- a/gas/config/tc-i386.c

>> +++ b/gas/config/tc-i386.c

>> @@ -6494,14 +6494,14 @@ duplicate:

>>             {

>>               /* Warn about some common errors, but press on regardless.

>>                  The first case can be generated by gcc (<= 2.8.1).  */

>> -             if (i.operands == 2)

>> +             if (i.operands == 2 && !i.op[0].regs->reg_type.bitfield.acc)

>>                 {

>>                   /* Reversed arguments on faddp, fsubp, etc.  */

>>                   as_warn (_("translating to `%s %s%s,%s%s'"), i.tm.name,

>>                            register_prefix, i.op[!intel_syntax].regs->reg_name,

>>                            register_prefix, i.op[intel_syntax].regs->reg_name);

>>                 }

>> -             else

>> +             else if (i.operands == 1)

>>                 {

>>                   /* Extraneous `l' suffix on fp insn.  */

>>                   as_warn (_("translating to `%s %s%s'"), i.tm.name,

> 

> Can we simply drop this encoding altogether?


Well, same here as for patch 1 of this series.

Jan
H.J. Lu March 7, 2018, 12:45 p.m. | #3
On Wed, Mar 7, 2018 at 4:38 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 07.03.18 at 13:32, <hjl.tools@gmail.com> wrote:

>> On Wed, Mar 7, 2018 at 1:48 AM, Jan Beulich <JBeulich@suse.com> wrote:

>>> --- a/gas/config/tc-i386.c

>>> +++ b/gas/config/tc-i386.c

>>> @@ -6494,14 +6494,14 @@ duplicate:

>>>             {

>>>               /* Warn about some common errors, but press on regardless.

>>>                  The first case can be generated by gcc (<= 2.8.1).  */

>>> -             if (i.operands == 2)

>>> +             if (i.operands == 2 && !i.op[0].regs->reg_type.bitfield.acc)

>>>                 {

>>>                   /* Reversed arguments on faddp, fsubp, etc.  */

>>>                   as_warn (_("translating to `%s %s%s,%s%s'"), i.tm.name,

>>>                            register_prefix, i.op[!intel_syntax].regs->reg_name,

>>>                            register_prefix, i.op[intel_syntax].regs->reg_name);

>>>                 }

>>> -             else

>>> +             else if (i.operands == 1)

>>>                 {

>>>                   /* Extraneous `l' suffix on fp insn.  */

>>>                   as_warn (_("translating to `%s %s%s'"), i.tm.name,

>>

>> Can we simply drop this encoding altogether?

>

> Well, same here as for patch 1 of this series.

>


If no one depends on them, we can simply drop them.


-- 
H.J.
Jan Beulich March 7, 2018, 12:54 p.m. | #4
>>> On 07.03.18 at 13:45, <hjl.tools@gmail.com> wrote:

> On Wed, Mar 7, 2018 at 4:38 AM, Jan Beulich <JBeulich@suse.com> wrote:

>>>>> On 07.03.18 at 13:32, <hjl.tools@gmail.com> wrote:

>>> On Wed, Mar 7, 2018 at 1:48 AM, Jan Beulich <JBeulich@suse.com> wrote:

>>>> --- a/gas/config/tc-i386.c

>>>> +++ b/gas/config/tc-i386.c

>>>> @@ -6494,14 +6494,14 @@ duplicate:

>>>>             {

>>>>               /* Warn about some common errors, but press on regardless.

>>>>                  The first case can be generated by gcc (<= 2.8.1).  */

>>>> -             if (i.operands == 2)

>>>> +             if (i.operands == 2 && !i.op[0].regs->reg_type.bitfield.acc)

>>>>                 {

>>>>                   /* Reversed arguments on faddp, fsubp, etc.  */

>>>>                   as_warn (_("translating to `%s %s%s,%s%s'"), i.tm.name,

>>>>                            register_prefix, i.op[!intel_syntax].regs->reg_name,

>>>>                            register_prefix, i.op[intel_syntax].regs->reg_name);

>>>>                 }

>>>> -             else

>>>> +             else if (i.operands == 1)

>>>>                 {

>>>>                   /* Extraneous `l' suffix on fp insn.  */

>>>>                   as_warn (_("translating to `%s %s%s'"), i.tm.name,

>>>

>>> Can we simply drop this encoding altogether?

>>

>> Well, same here as for patch 1 of this series.

>>

> 

> If no one depends on them, we can simply drop them.


But how do we know? Dropping thing is much more risky a change
than fixing what's currently wrong.

Jan

Patch

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -6494,14 +6494,14 @@  duplicate:
 	    {
 	      /* Warn about some common errors, but press on regardless.
 		 The first case can be generated by gcc (<= 2.8.1).  */
-	      if (i.operands == 2)
+	      if (i.operands == 2 && !i.op[0].regs->reg_type.bitfield.acc)
 		{
 		  /* Reversed arguments on faddp, fsubp, etc.  */
 		  as_warn (_("translating to `%s %s%s,%s%s'"), i.tm.name,
 			   register_prefix, i.op[!intel_syntax].regs->reg_name,
 			   register_prefix, i.op[intel_syntax].regs->reg_name);
 		}
-	      else
+	      else if (i.operands == 1)
 		{
 		  /* Extraneous `l' suffix on fp insn.  */
 		  as_warn (_("translating to `%s %s%s'"), i.tm.name,
--- a/gas/testsuite/gas/i386/fsubp.l
+++ b/gas/testsuite/gas/i386/fsubp.l
@@ -7,16 +7,12 @@ 
 .*:13: Warning:.*
 .*:15: Warning:.*
 .*:16: Warning:.*
-.*:21: Warning:.*
 .*:22: Warning:.*
 .*:23: Warning:.*
-.*:24: Warning:.*
 .*:25: Warning:.*
 .*:26: Warning:.*
-.*:28: Warning:.*
 .*:29: Warning:.*
 .*:30: Warning:.*
-.*:31: Warning:.*
 .*:32: Warning:.*
 .*:33: Warning:.*
 GAS LISTING .*
@@ -51,26 +47,22 @@  GAS LISTING .*
   19              		\.att_mnemonic
   20              	div_i:
   21 0018 DEF8     		fdivp	st, st
-.*Warning:.*
   22 001a DEF9     		fdivp	st, st\(1\)
 .*Warning:.*
   23 001c DEFA     		fdivp	st, st\(2\)
 .*Warning:.*
   24 001e DEF0     		fdivrp	st, st
-.*Warning:.*
   25 0020 DEF1     		fdivrp	st, st\(1\)
 .*Warning:.*
   26 0022 DEF2     		fdivrp	st, st\(2\)
 .*Warning:.*
   27              	sub_i:
   28 0024 DEE8     		fsubp	st, st
-.*Warning:.*
   29 0026 DEE9     		fsubp	st, st\(1\)
 .*Warning:.*
   30 0028 DEEA     		fsubp	st, st\(2\)
 .*Warning:.*
   31 002a DEE0     		fsubrp	st, st
-.*Warning:.*
   32 002c DEE1     		fsubrp	st, st\(1\)
 .*Warning:.*
   33 002e DEE2     		fsubrp	st, st\(2\)