x86: Replace evex-no-scale.s with evex-no-scale-[32|64].s

Message ID 20180810172814.7966-1-hjl.tools@gmail.com
State New
Headers show
Series
  • x86: Replace evex-no-scale.s with evex-no-scale-[32|64].s
Related show

Commit Message

H.J. Lu Aug. 10, 2018, 5:28 p.m.
.if is_64bit
	vmovaps	-1024(%rip), %zmm0
	vmovaps	64(,%rax), %zmm0
	vmovaps	64(,%riz), %zmm0
.endif

doesn't with i686-elf cross binutils on 64-bit hosts:

evex-no-scale.s: Assembler messages:
evex-no-scale.s:10: Error: bad register name `%rip)'
evex-no-scale.s:11: Error: bad register name `%rax)'
evex-no-scale.s:12: Error: bad register name `%riz)'

This patch replaces evex-no-scale.s with evex-no-scale-32.s and
evex-no-scale-64.s.

	* testsuite/gas/i386/evex-no-scale-32.d: Don't use
	evex-no-scale.s.
	* testsuite/gas/i386/evex-no-scale-64.d: Likewise.
	* testsuite/gas/i386/evex-no-scale-32.s: New file.
	* testsuite/gas/i386/evex-no-scale-64.s: Likewise.
	* testsuite/gas/i386/evex-no-scale.s: Removed.
---
 gas/ChangeLog                                          |  9 +++++++++
 gas/testsuite/gas/i386/evex-no-scale-32.d              |  1 -
 gas/testsuite/gas/i386/evex-no-scale-32.s              |  7 +++++++
 gas/testsuite/gas/i386/evex-no-scale-64.d              |  1 -
 .../gas/i386/{evex-no-scale.s => evex-no-scale-64.s}   | 10 ----------
 5 files changed, 16 insertions(+), 12 deletions(-)
 create mode 100644 gas/testsuite/gas/i386/evex-no-scale-32.s
 rename gas/testsuite/gas/i386/{evex-no-scale.s => evex-no-scale-64.s} (52%)

-- 
2.17.1

Comments

H.J. Lu Aug. 10, 2018, 7:58 p.m. | #1
On Fri, Aug 10, 2018 at 10:28 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> .if is_64bit

>         vmovaps -1024(%rip), %zmm0

>         vmovaps 64(,%rax), %zmm0

>         vmovaps 64(,%riz), %zmm0

> .endif

>

> doesn't with i686-elf cross binutils on 64-bit hosts:

>

> evex-no-scale.s: Assembler messages:

> evex-no-scale.s:10: Error: bad register name `%rip)'

> evex-no-scale.s:11: Error: bad register name `%rax)'

> evex-no-scale.s:12: Error: bad register name `%riz)'

>

> This patch replaces evex-no-scale.s with evex-no-scale-32.s and

> evex-no-scale-64.s.

>

>         * testsuite/gas/i386/evex-no-scale-32.d: Don't use

>         evex-no-scale.s.

>         * testsuite/gas/i386/evex-no-scale-64.d: Likewise.

>         * testsuite/gas/i386/evex-no-scale-32.s: New file.

>         * testsuite/gas/i386/evex-no-scale-64.s: Likewise.

>         * testsuite/gas/i386/evex-no-scale.s: Removed.


I backported it to 2.31 branch.


-- 
H.J.
Jan Beulich Aug. 13, 2018, 6:20 a.m. | #2
>>> On 10.08.18 at 19:28, <hjl.tools@gmail.com> wrote:

> .if is_64bit

> 	vmovaps	-1024(%rip), %zmm0

> 	vmovaps	64(,%rax), %zmm0

> 	vmovaps	64(,%riz), %zmm0

> .endif

> 

> doesn't with i686-elf cross binutils on 64-bit hosts:

> 

> evex-no-scale.s: Assembler messages:

> evex-no-scale.s:10: Error: bad register name `%rip)'

> evex-no-scale.s:11: Error: bad register name `%rax)'

> evex-no-scale.s:12: Error: bad register name `%riz)'


Mind shedding some light on the _actual_ issue? The fact that
the 64-bit code won't assemble with a 32-bit assembler doesn't
come as a surprise. The question is why is_64bit is set in this
case. I was actually meaning to use this approach in further
tests, to reduce the split / redundancy between similar or even
close to identical 32- and 64-bit tests.

Jan
H.J. Lu Aug. 13, 2018, 11:29 a.m. | #3
On Sun, Aug 12, 2018 at 11:20 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 10.08.18 at 19:28, <hjl.tools@gmail.com> wrote:

>> .if is_64bit

>>       vmovaps -1024(%rip), %zmm0

>>       vmovaps 64(,%rax), %zmm0

>>       vmovaps 64(,%riz), %zmm0

>> .endif

>>

>> doesn't with i686-elf cross binutils on 64-bit hosts:

>>

>> evex-no-scale.s: Assembler messages:

>> evex-no-scale.s:10: Error: bad register name `%rip)'

>> evex-no-scale.s:11: Error: bad register name `%rax)'

>> evex-no-scale.s:12: Error: bad register name `%riz)'

>

> Mind shedding some light on the _actual_ issue? The fact that

> the 64-bit code won't assemble with a 32-bit assembler doesn't

> come as a surprise. The question is why is_64bit is set in this

> case. I was actually meaning to use this approach in further

> tests, to reduce the split / redundancy between similar or even

> close to identical 32- and 64-bit tests.

>


I didn't investigate further.  You can try it yourself.


-- 
H.J.
Jan Beulich Aug. 13, 2018, 12:01 p.m. | #4
>>> On 13.08.18 at 13:29, <hjl.tools@gmail.com> wrote:

> On Sun, Aug 12, 2018 at 11:20 PM, Jan Beulich <JBeulich@suse.com> wrote:

>>>>> On 10.08.18 at 19:28, <hjl.tools@gmail.com> wrote:

>>> .if is_64bit

>>>       vmovaps -1024(%rip), %zmm0

>>>       vmovaps 64(,%rax), %zmm0

>>>       vmovaps 64(,%riz), %zmm0

>>> .endif

>>>

>>> doesn't with i686-elf cross binutils on 64-bit hosts:

>>>

>>> evex-no-scale.s: Assembler messages:

>>> evex-no-scale.s:10: Error: bad register name `%rip)'

>>> evex-no-scale.s:11: Error: bad register name `%rax)'

>>> evex-no-scale.s:12: Error: bad register name `%riz)'

>>

>> Mind shedding some light on the _actual_ issue? The fact that

>> the 64-bit code won't assemble with a 32-bit assembler doesn't

>> come as a surprise. The question is why is_64bit is set in this

>> case. I was actually meaning to use this approach in further

>> tests, to reduce the split / redundancy between similar or even

>> close to identical 32- and 64-bit tests.

> 

> I didn't investigate further.  You can try it yourself.


Well, I'll try to remember to do so eventually, but I'm sorry - this is
not a reasonable way to fix a test suite issue in my opinion. You
should first understand what the issue is, and only on that basis
decide how to best address the issue. I'd really like to ask you to
revert your "fix" given this situation. This is even worse than
requiring test sources to all have bogus .align directives at their
ends.

Jan
H.J. Lu Aug. 13, 2018, 12:37 p.m. | #5
On Mon, Aug 13, 2018 at 5:01 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 13.08.18 at 13:29, <hjl.tools@gmail.com> wrote:

>> On Sun, Aug 12, 2018 at 11:20 PM, Jan Beulich <JBeulich@suse.com> wrote:

>>>>>> On 10.08.18 at 19:28, <hjl.tools@gmail.com> wrote:

>>>> .if is_64bit

>>>>       vmovaps -1024(%rip), %zmm0

>>>>       vmovaps 64(,%rax), %zmm0

>>>>       vmovaps 64(,%riz), %zmm0

>>>> .endif

>>>>

>>>> doesn't with i686-elf cross binutils on 64-bit hosts:

>>>>

>>>> evex-no-scale.s: Assembler messages:

>>>> evex-no-scale.s:10: Error: bad register name `%rip)'

>>>> evex-no-scale.s:11: Error: bad register name `%rax)'

>>>> evex-no-scale.s:12: Error: bad register name `%riz)'

>>>

>>> Mind shedding some light on the _actual_ issue? The fact that

>>> the 64-bit code won't assemble with a 32-bit assembler doesn't

>>> come as a surprise. The question is why is_64bit is set in this

>>> case. I was actually meaning to use this approach in further

>>> tests, to reduce the split / redundancy between similar or even

>>> close to identical 32- and 64-bit tests.

>>

>> I didn't investigate further.  You can try it yourself.

>

> Well, I'll try to remember to do so eventually, but I'm sorry - this is

> not a reasonable way to fix a test suite issue in my opinion. You

> should first understand what the issue is, and only on that basis

> decide how to best address the issue. I'd really like to ask you to

> revert your "fix" given this situation. This is even worse than

> requiring test sources to all have bogus .align directives at their

> ends.


"make check" result for "i686-elf" target was clean before that
testcase was added.  This is a less intrusive way to fix it on master
and backport it to 2.31 branch.


-- 
H.J.
Jan Beulich Aug. 13, 2018, 12:52 p.m. | #6
>>> On 13.08.18 at 14:37, <hjl.tools@gmail.com> wrote:

> On Mon, Aug 13, 2018 at 5:01 AM, Jan Beulich <JBeulich@suse.com> wrote:

>>>>> On 13.08.18 at 13:29, <hjl.tools@gmail.com> wrote:

>>> On Sun, Aug 12, 2018 at 11:20 PM, Jan Beulich <JBeulich@suse.com> wrote:

>>>>>>> On 10.08.18 at 19:28, <hjl.tools@gmail.com> wrote:

>>>>> .if is_64bit

>>>>>       vmovaps -1024(%rip), %zmm0

>>>>>       vmovaps 64(,%rax), %zmm0

>>>>>       vmovaps 64(,%riz), %zmm0

>>>>> .endif

>>>>>

>>>>> doesn't with i686-elf cross binutils on 64-bit hosts:

>>>>>

>>>>> evex-no-scale.s: Assembler messages:

>>>>> evex-no-scale.s:10: Error: bad register name `%rip)'

>>>>> evex-no-scale.s:11: Error: bad register name `%rax)'

>>>>> evex-no-scale.s:12: Error: bad register name `%riz)'

>>>>

>>>> Mind shedding some light on the _actual_ issue? The fact that

>>>> the 64-bit code won't assemble with a 32-bit assembler doesn't

>>>> come as a surprise. The question is why is_64bit is set in this

>>>> case. I was actually meaning to use this approach in further

>>>> tests, to reduce the split / redundancy between similar or even

>>>> close to identical 32- and 64-bit tests.

>>>

>>> I didn't investigate further.  You can try it yourself.

>>

>> Well, I'll try to remember to do so eventually, but I'm sorry - this is

>> not a reasonable way to fix a test suite issue in my opinion. You

>> should first understand what the issue is, and only on that basis

>> decide how to best address the issue. I'd really like to ask you to

>> revert your "fix" given this situation. This is even worse than

>> requiring test sources to all have bogus .align directives at their

>> ends.

> 

> "make check" result for "i686-elf" target was clean before that

> testcase was added.  This is a less intrusive way to fix it on master

> and backport it to 2.31 branch.


That's all fine, but if for a 32-bit target "inc %eax" does not produce
a 1-byte encoding, then there's quite likely something broken
elsewhere. Your change hides this brokenness rather than keeping it
exposed until it's fixed (or until it's understood that this is expected
and my assumptions are wrong upon which I've built the test case).

Jan

Patch

diff --git a/gas/ChangeLog b/gas/ChangeLog
index ba6d9ca789..2df3fdb4e7 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,12 @@ 
+2018-08-10  H.J. Lu  <hongjiu.lu@intel.com>
+
+	* testsuite/gas/i386/evex-no-scale.s: Removed.
+	* testsuite/gas/i386/evex-no-scale-32.d: Don't use
+	evex-no-scale.s.
+	* testsuite/gas/i386/evex-no-scale-64.d: Likewise.
+	* testsuite/gas/i386/evex-no-scale-32.s: New file.
+	* testsuite/gas/i386/evex-no-scale-64.s: Likewise.
+
 2018-08-09  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* as.c (show_usage): Display default option for --elf-stt-common=.
diff --git a/gas/testsuite/gas/i386/evex-no-scale-32.d b/gas/testsuite/gas/i386/evex-no-scale-32.d
index e6116bc592..0a2860d32e 100644
--- a/gas/testsuite/gas/i386/evex-no-scale-32.d
+++ b/gas/testsuite/gas/i386/evex-no-scale-32.d
@@ -1,4 +1,3 @@ 
-#source: evex-no-scale.s
 #objdump: -dw
 #name: ix86 EVEX no disp scaling
 
diff --git a/gas/testsuite/gas/i386/evex-no-scale-32.s b/gas/testsuite/gas/i386/evex-no-scale-32.s
new file mode 100644
index 0000000000..e28c73f2a3
--- /dev/null
+++ b/gas/testsuite/gas/i386/evex-no-scale-32.s
@@ -0,0 +1,7 @@ 
+	.allow_index_reg
+	.text
+disp:
+	vmovaps	64(,%eax), %zmm0
+	vmovaps	64(,%eiz), %zmm0
+	vmovaps	64, %zmm0
+	addr16 vmovaps 64, %zmm0
diff --git a/gas/testsuite/gas/i386/evex-no-scale-64.d b/gas/testsuite/gas/i386/evex-no-scale-64.d
index b66f15bf8b..d52d947ab3 100644
--- a/gas/testsuite/gas/i386/evex-no-scale-64.d
+++ b/gas/testsuite/gas/i386/evex-no-scale-64.d
@@ -1,4 +1,3 @@ 
-#source: evex-no-scale.s
 #objdump: -dw
 #name: x86-64 EVEX no disp scaling
 
diff --git a/gas/testsuite/gas/i386/evex-no-scale.s b/gas/testsuite/gas/i386/evex-no-scale-64.s
similarity index 52%
rename from gas/testsuite/gas/i386/evex-no-scale.s
rename to gas/testsuite/gas/i386/evex-no-scale-64.s
index ed0f2312af..bc3749cc0c 100644
--- a/gas/testsuite/gas/i386/evex-no-scale.s
+++ b/gas/testsuite/gas/i386/evex-no-scale-64.s
@@ -1,19 +1,9 @@ 
 	.allow_index_reg
-	.section .probe, "", @progbits
-.Lprobe_64bit:
-	inc	%eax
-.equiv is_64bit, (. - .Lprobe_64bit) / 2
-
 	.text
 disp:
-.if is_64bit
 	vmovaps	-1024(%rip), %zmm0
 	vmovaps	64(,%rax), %zmm0
 	vmovaps	64(,%riz), %zmm0
-.endif
 	vmovaps	64(,%eax), %zmm0
 	vmovaps	64(,%eiz), %zmm0
 	vmovaps	64, %zmm0
-.if !is_64bit
-	addr16 vmovaps 64, %zmm0
-.endif