[1/4] x86: correctly encode FSUBP

Message ID 5A9FC35F02000078001AF42D@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:47 a.m.
While the combination of Intel syntax and AT&T mnemonics is bogus
anyway, we nevertheless shouldn't mis-encode such insns.

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

	* testsuite/gas/i386/fsubp.l, testsuite/gas/i386/fsubp.s: New.
	* testsuite/gas/i386/i386.exp: Run new test.

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

	* i386-opc.tbl (fsubp): Correct encoding.
	* i386-tlb.h: Re-generate.

Comments

H.J. Lu March 7, 2018, 12:30 p.m. | #1
On Wed, Mar 7, 2018 at 1:47 AM, Jan Beulich <JBeulich@suse.com> wrote:
> While the combination of Intel syntax and AT&T mnemonics is bogus

> anyway, we nevertheless shouldn't mis-encode such insns.

>

> gas/

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

>

>         * testsuite/gas/i386/fsubp.l, testsuite/gas/i386/fsubp.s: New.

>         * testsuite/gas/i386/i386.exp: Run new test.

>

> opcodes/

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

>

>         * i386-opc.tbl (fsubp): Correct encoding.

>         * i386-tlb.h: Re-generate.

>


I assume that no one uses it.  Can't we simply drop it?

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

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

>> While the combination of Intel syntax and AT&T mnemonics is bogus

>> anyway, we nevertheless shouldn't mis-encode such insns.

>>

>> gas/

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

>>

>>         * testsuite/gas/i386/fsubp.l, testsuite/gas/i386/fsubp.s: New.

>>         * testsuite/gas/i386/i386.exp: Run new test.

>>

>> opcodes/

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

>>

>>         * i386-opc.tbl (fsubp): Correct encoding.

>>         * i386-tlb.h: Re-generate.

>>

> 

> I assume that no one uses it.  Can't we simply drop it?


I don't know, and would dare to chance it. But perhaps you know
better than me. Of course, if we drop this one, there are others to
be dropped at the same time.

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

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

>>> While the combination of Intel syntax and AT&T mnemonics is bogus

>>> anyway, we nevertheless shouldn't mis-encode such insns.

>>>

>>> gas/

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

>>>

>>>         * testsuite/gas/i386/fsubp.l, testsuite/gas/i386/fsubp.s: New.

>>>         * testsuite/gas/i386/i386.exp: Run new test.

>>>

>>> opcodes/

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

>>>

>>>         * i386-opc.tbl (fsubp): Correct encoding.

>>>         * i386-tlb.h: Re-generate.

>>>

>>

>> I assume that no one uses it.  Can't we simply drop it?

>

> I don't know, and would dare to chance it. But perhaps you know

> better than me. Of course, if we drop this one, there are others to

> be dropped at the same time.

>


We are changing their encodings.  If we can't drop them, it means
that someone is depending on their old encodings.

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

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

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

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

>>>> While the combination of Intel syntax and AT&T mnemonics is bogus

>>>> anyway, we nevertheless shouldn't mis-encode such insns.

>>>>

>>>> gas/

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

>>>>

>>>>         * testsuite/gas/i386/fsubp.l, testsuite/gas/i386/fsubp.s: New.

>>>>         * testsuite/gas/i386/i386.exp: Run new test.

>>>>

>>>> opcodes/

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

>>>>

>>>>         * i386-opc.tbl (fsubp): Correct encoding.

>>>>         * i386-tlb.h: Re-generate.

>>>>

>>>

>>> I assume that no one uses it.  Can't we simply drop it?

>>

>> I don't know, and would dare to chance it. But perhaps you know

>> better than me. Of course, if we drop this one, there are others to

>> be dropped at the same time.

> 

> We are changing their encodings.  If we can't drop them, it means

> that someone is depending on their old encodings.


Well, no, not exactly: If we do as you suggest, we should remove
more than just this broken encoding. People may use some of the
others, or may have happened to use the one here only with odd
numbered registers. But again - I'm not entirely opposed to dropping
the whole OldGcc logic, it's just that (at least at this point in time) it's
not going to be me to do it. Hence rather than leaving things broken,
I'd prefer to fix them. (As a side note, patch 3 and maybe also patch
4 of this series would probably need changing, just like a number of
existing test cases.)

Jan
H.J. Lu March 7, 2018, 1:26 p.m. | #5
On Wed, Mar 7, 2018 at 4:54 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 07.03.18 at 13:39, <hjl.tools@gmail.com> wrote:

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

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

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

>>>>> While the combination of Intel syntax and AT&T mnemonics is bogus

>>>>> anyway, we nevertheless shouldn't mis-encode such insns.

>>>>>

>>>>> gas/

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

>>>>>

>>>>>         * testsuite/gas/i386/fsubp.l, testsuite/gas/i386/fsubp.s: New.

>>>>>         * testsuite/gas/i386/i386.exp: Run new test.

>>>>>

>>>>> opcodes/

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

>>>>>

>>>>>         * i386-opc.tbl (fsubp): Correct encoding.

>>>>>         * i386-tlb.h: Re-generate.

>>>>>

>>>>

>>>> I assume that no one uses it.  Can't we simply drop it?

>>>

>>> I don't know, and would dare to chance it. But perhaps you know

>>> better than me. Of course, if we drop this one, there are others to

>>> be dropped at the same time.

>>

>> We are changing their encodings.  If we can't drop them, it means

>> that someone is depending on their old encodings.

>

> Well, no, not exactly: If we do as you suggest, we should remove

> more than just this broken encoding. People may use some of the

> others, or may have happened to use the one here only with odd

> numbered registers. But again - I'm not entirely opposed to dropping

> the whole OldGcc logic, it's just that (at least at this point in time) it's

> not going to be me to do it. Hence rather than leaving things broken,

> I'd prefer to fix them. (As a side note, patch 3 and maybe also patch

> 4 of this series would probably need changing, just like a number of

> existing test cases.)

>


We should drop the broken encoding.  If people do use them, they
will get an assembler error.  Otherwise, they will get a run-time
error which is much harder to track down.


-- 
H.J.
Jan Beulich March 7, 2018, 1:54 p.m. | #6
>>> On 07.03.18 at 14:26, <hjl.tools@gmail.com> wrote:

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

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

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

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

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

>>>>>> While the combination of Intel syntax and AT&T mnemonics is bogus

>>>>>> anyway, we nevertheless shouldn't mis-encode such insns.

>>>>>>

>>>>>> gas/

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

>>>>>>

>>>>>>         * testsuite/gas/i386/fsubp.l, testsuite/gas/i386/fsubp.s: New.

>>>>>>         * testsuite/gas/i386/i386.exp: Run new test.

>>>>>>

>>>>>> opcodes/

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

>>>>>>

>>>>>>         * i386-opc.tbl (fsubp): Correct encoding.

>>>>>>         * i386-tlb.h: Re-generate.

>>>>>>

>>>>>

>>>>> I assume that no one uses it.  Can't we simply drop it?

>>>>

>>>> I don't know, and would dare to chance it. But perhaps you know

>>>> better than me. Of course, if we drop this one, there are others to

>>>> be dropped at the same time.

>>>

>>> We are changing their encodings.  If we can't drop them, it means

>>> that someone is depending on their old encodings.

>>

>> Well, no, not exactly: If we do as you suggest, we should remove

>> more than just this broken encoding. People may use some of the

>> others, or may have happened to use the one here only with odd

>> numbered registers. But again - I'm not entirely opposed to dropping

>> the whole OldGcc logic, it's just that (at least at this point in time) it's

>> not going to be me to do it. Hence rather than leaving things broken,

>> I'd prefer to fix them. (As a side note, patch 3 and maybe also patch

>> 4 of this series would probably need changing, just like a number of

>> existing test cases.)

> 

> We should drop the broken encoding.  If people do use them, they

> will get an assembler error.  Otherwise, they will get a run-time

> error which is much harder to track down.


Well, again - we should drop all that OldGcc cruft, or keep it and fix
it. Leaving something broken around is simply bad, and leaving OldGcc
partially implemented isn't much better.

Jan
H.J. Lu March 7, 2018, 2:08 p.m. | #7
On Wed, Mar 7, 2018 at 5:54 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 07.03.18 at 14:26, <hjl.tools@gmail.com> wrote:

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

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

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

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

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

>>>>>>> While the combination of Intel syntax and AT&T mnemonics is bogus

>>>>>>> anyway, we nevertheless shouldn't mis-encode such insns.

>>>>>>>

>>>>>>> gas/

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

>>>>>>>

>>>>>>>         * testsuite/gas/i386/fsubp.l, testsuite/gas/i386/fsubp.s: New.

>>>>>>>         * testsuite/gas/i386/i386.exp: Run new test.

>>>>>>>

>>>>>>> opcodes/

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

>>>>>>>

>>>>>>>         * i386-opc.tbl (fsubp): Correct encoding.

>>>>>>>         * i386-tlb.h: Re-generate.

>>>>>>>

>>>>>>

>>>>>> I assume that no one uses it.  Can't we simply drop it?

>>>>>

>>>>> I don't know, and would dare to chance it. But perhaps you know

>>>>> better than me. Of course, if we drop this one, there are others to

>>>>> be dropped at the same time.

>>>>

>>>> We are changing their encodings.  If we can't drop them, it means

>>>> that someone is depending on their old encodings.

>>>

>>> Well, no, not exactly: If we do as you suggest, we should remove

>>> more than just this broken encoding. People may use some of the

>>> others, or may have happened to use the one here only with odd

>>> numbered registers. But again - I'm not entirely opposed to dropping

>>> the whole OldGcc logic, it's just that (at least at this point in time) it's

>>> not going to be me to do it. Hence rather than leaving things broken,

>>> I'd prefer to fix them. (As a side note, patch 3 and maybe also patch

>>> 4 of this series would probably need changing, just like a number of

>>> existing test cases.)

>>

>> We should drop the broken encoding.  If people do use them, they

>> will get an assembler error.  Otherwise, they will get a run-time

>> error which is much harder to track down.

>

> Well, again - we should drop all that OldGcc cruft, or keep it and fix

> it. Leaving something broken around is simply bad, and leaving OldGcc

> partially implemented isn't much better.

>


Let me work on dropping OldGcc.


-- 
H.J.

Patch

--- /dev/null
+++ b/gas/testsuite/gas/i386/fsubp.l
@@ -0,0 +1,77 @@ 
+.*: Assembler messages:
+.*:5: Warning:.*
+.*:6: Warning:.*
+.*:8: Warning:.*
+.*:9: Warning:.*
+.*:12: Warning:.*
+.*: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 .*
+
+
+   1              		\.text
+   2              		\.att_mnemonic
+   3              	div_a:
+   4 0000 DEF0     		fdivp	%st, %st
+   5 0002 DEF1     		fdivp	%st\(1\), %st
+.*Warning:.*
+   6 0004 DEF2     		fdivp	%st\(2\), %st
+.*Warning:.*
+   7 0006 DEF8     		fdivrp	%st, %st
+   8 0008 DEF9     		fdivrp	%st\(1\), %st
+.*Warning:.*
+   9 000a DEFA     		fdivrp	%st\(2\), %st
+.*Warning:.*
+  10              	sub_a:
+  11 000c DEE0     		fsubp	%st, %st
+  12 000e DEE1     		fsubp	%st\(1\), %st
+.*Warning:.*
+  13 0010 DEE2     		fsubp	%st\(2\), %st
+.*Warning:.*
+  14 0012 DEE8     		fsubrp	%st, %st
+  15 0014 DEE9     		fsubrp	%st\(1\), %st
+.*Warning:.*
+  16 0016 DEEA     		fsubrp	%st\(2\), %st
+.*Warning:.*
+  17              	
+  18              		\.intel_syntax noprefix
+  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\)
+.*Warning:.*
--- /dev/null
+++ b/gas/testsuite/gas/i386/fsubp.s
@@ -0,0 +1,33 @@ 
+	.text
+	.att_mnemonic
+div_a:
+	fdivp	%st, %st
+	fdivp	%st(1), %st
+	fdivp	%st(2), %st
+	fdivrp	%st, %st
+	fdivrp	%st(1), %st
+	fdivrp	%st(2), %st
+sub_a:
+	fsubp	%st, %st
+	fsubp	%st(1), %st
+	fsubp	%st(2), %st
+	fsubrp	%st, %st
+	fsubrp	%st(1), %st
+	fsubrp	%st(2), %st
+
+	.intel_syntax noprefix
+	.att_mnemonic
+div_i:
+	fdivp	st, st
+	fdivp	st, st(1)
+	fdivp	st, st(2)
+	fdivrp	st, st
+	fdivrp	st, st(1)
+	fdivrp	st, st(2)
+sub_i:
+	fsubp	st, st
+	fsubp	st, st(1)
+	fsubp	st, st(2)
+	fsubrp	st, st
+	fsubrp	st, st(1)
+	fsubrp	st, st(2)
--- a/gas/testsuite/gas/i386/i386.exp
+++ b/gas/testsuite/gas/i386/i386.exp
@@ -40,6 +40,7 @@  if [expr ([istarget "i*86-*-*"] ||  [ist
     set ASFLAGS "$ASFLAGS --32"
 
     run_list_test "float" "-al -mmnemonic=att"
+    run_list_test "fsubp" "-al"
     run_list_test "general" "-al --listing-lhs-width=2 -mold-gcc"
     run_list_test "inval" "-al"
     run_list_test "inval-16" "-al"
--- a/opcodes/i386-opc.tbl
+++ b/opcodes/i386-opc.tbl
@@ -632,7 +632,7 @@  fsubp, 2, 0xdee0, None, 2, CpuFP, ShortF
 fsubp, 1, 0xdee0, None, 2, CpuFP, ShortForm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|ATTMnemonic|ATTSyntax, { FloatReg }
 fsubp, 0, 0xdee1, None, 2, CpuFP, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|ATTMnemonic|ATTSyntax, { 0 }
 fsubp, 2, 0xdee0, None, 2, CpuFP, ShortForm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|Ugh|ATTMnemonic|ATTSyntax|OldGcc, { FloatReg, FloatAcc }
-fsubp, 2, 0xdee9, None, 2, CpuFP, ShortForm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|Ugh|ATTMnemonic|OldGcc, { FloatReg, FloatAcc }
+fsubp, 2, 0xdee8, None, 2, CpuFP, ShortForm|No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|Ugh|ATTMnemonic|OldGcc, { FloatReg, FloatAcc }
 fsubp, 2, 0xdee8, None, 2, CpuFP, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|No_qSuf|ShortForm, { FloatAcc, FloatReg }
 fsubp, 1, 0xdee8, None, 2, CpuFP, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_ldSuf|No_qSuf|ShortForm, { FloatReg }
 fsubp, 0, 0xdee9, None, 2, CpuFP, No_bSuf|No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf, { 0 }