[GAS,ARM] Fix UDF testism

Message ID 5BC9F16E.3050104@arm.com
State New
Headers show
Series
  • [GAS,ARM] Fix UDF testism
Related show

Commit Message

Andre Vieira (lists) Oct. 19, 2018, 2:59 p.m.
Hi,

This patch fixes a testism.
The old test never checked the objdump output since the 'error-output'
directive
would exit and thus never run objdump.  When this test was changed to
adhere to
use the new warning_output we started to run objdump.  The expected objdump
output was old and had bitrotten, this fixes the layout, as the
"disassembly"
itself did not change.

Is this OK for trunk?

2018-10-19  Andre Vieira  <andre.simoesdiasvieira@arm.com>

        * testsuite/gas/arm/udf.d: Update expected output.

Comments

Thomas Preudhomme Oct. 26, 2018, 10:24 a.m. | #1
Hi Andre,

I'm not a maintainer but I have one comment nonetheless: I'd suggest
using the 0+ notation to absorb all the 0s in the address prefix to be
consistent with the rest of the testsuite (see eg.
gas/testsuite/gas/arm/arch6zk.d as a random example).

Looks good to me otherwise. Best regards,

Thomas
On Fri, 19 Oct 2018 at 16:00, Andre Vieira (lists)
<Andre.SimoesDiasVieira@arm.com> wrote:
>

> Hi,

>

> This patch fixes a testism.

> The old test never checked the objdump output since the 'error-output'

> directive

> would exit and thus never run objdump.  When this test was changed to

> adhere to

> use the new warning_output we started to run objdump.  The expected objdump

> output was old and had bitrotten, this fixes the layout, as the

> "disassembly"

> itself did not change.

>

> Is this OK for trunk?

>

> 2018-10-19  Andre Vieira  <andre.simoesdiasvieira@arm.com>

>

>         * testsuite/gas/arm/udf.d: Update expected output.
Andre Vieira (lists) Oct. 26, 2018, 2:15 p.m. | #2
Hi Thomas,

Thanks for the review.  I changed the test file and used the same as I
did in another test, '[^<]*<', that way it also consumes the address
which is irrelevant to this test.

Cheers,
Andre
On 26/10/18 11:24, Thomas Preudhomme wrote:
> Hi Andre,

> 

> I'm not a maintainer but I have one comment nonetheless: I'd suggest

> using the 0+ notation to absorb all the 0s in the address prefix to be

> consistent with the rest of the testsuite (see eg.

> gas/testsuite/gas/arm/arch6zk.d as a random example).

> 

> Looks good to me otherwise. Best regards,

> 

> Thomas

> On Fri, 19 Oct 2018 at 16:00, Andre Vieira (lists)

> <Andre.SimoesDiasVieira@arm.com> wrote:

>>

>> Hi,

>>

>> This patch fixes a testism.

>> The old test never checked the objdump output since the 'error-output'

>> directive

>> would exit and thus never run objdump.  When this test was changed to

>> adhere to

>> use the new warning_output we started to run objdump.  The expected objdump

>> output was old and had bitrotten, this fixes the layout, as the

>> "disassembly"

>> itself did not change.

>>

>> Is this OK for trunk?

>>

>> 2018-10-19  Andre Vieira  <andre.simoesdiasvieira@arm.com>

>>

>>         * testsuite/gas/arm/udf.d: Update expected output.
diff --git a/gas/testsuite/gas/arm/udf.d b/gas/testsuite/gas/arm/udf.d
index c8777a9f052e71ad679a27c64cf666afa1dcf38f..ef24209aec27d0440f8109dc27d9e357c8a51b9f 100644
--- a/gas/testsuite/gas/arm/udf.d
+++ b/gas/testsuite/gas/arm/udf.d
@@ -6,26 +6,22 @@
 .*: +file format .*arm.*
 
 Disassembly of section \.text:
-
-0+0 <arm>:
-\s*0:\s+e7f000f0\s+udf	#0
-\s*4:\s+e7fabcfd\s+udf	#43981	; 0xabcd
-
-0+0 <thumb>:
-\s*8:\s+deab\s+udf	#171	; 0xab
-\s*a:\s+decd\s+udf	#205	; 0xcd
-\s*c:\s+de00\s+udf	#0
-\s*e:\s+46c0\s+nop.*
-\s*10:\s+f7f0 a000\s+udf\.w	#0
-\s*14:\s+f7f1 a234\s+udf\.w	#4660	; 0x1234
-\s*18:\s+f7fc acdd\s+udf\.w	#52445	; 0xccdd
-\s*1c:\s+bf08\s+it	eq
-\s*1e:\s+de12\s+udfeq	#18
-\s*20:\s+de23\s+udf	#35	; 0x23
-\s*22:\s+de34\s+udf	#52	; 0x34
-\s*24:\s+de56\s+udf	#86	; 0x56
-\s*26:\s+bf18\s+it	ne
-\s*28:\s+f7f1 a234\s+udfne\.w	#4660	; 0x1234
-\s*2c:\s+f7f2 a345\s+udf\.w	#9029	; 0x2345
-\s*30:\s+f7f3 a456\s+udf\.w	#13398	; 0x3456
-\s*34:\s+f7f5 a678\s+udf\.w	#22136	; 0x5678
+[^<]*<arm> e7f000f0 	udf	#0
+[^<]*<arm\+0x4> e7fabcfd 	udf	#43981	; 0xabcd
+[^<]*<thumb> deab      	udf	#171	; 0xab
+[^<]*<thumb\+0x2> decd      	udf	#205	; 0xcd
+[^<]*<thumb\+0x4> de00      	udf	#0
+[^<]*<thumb\+0x6> bf00      	nop
+[^<]*<thumb\+0x8> f7f0 a000 	udf.w	#0
+[^<]*<thumb\+0xc> f7f1 a234 	udf.w	#4660	; 0x1234
+[^<]*<thumb\+0x10> f7fc acdd 	udf.w	#52445	; 0xccdd
+[^<]*<thumb\+0x14> bf08      	it	eq
+[^<]*<thumb\+0x16> de12      	udfeq	#18
+[^<]*<thumb\+0x18> de23      	udf	#35	; 0x23
+[^<]*<thumb\+0x1a> de34      	udf	#52	; 0x34
+[^<]*<thumb\+0x1c> de56      	udf	#86	; 0x56
+[^<]*<thumb\+0x1e> bf18      	it	ne
+[^<]*<thumb\+0x20> f7f1 a234 	udfne.w	#4660	; 0x1234
+[^<]*<thumb\+0x24> f7f2 a345 	udf.w	#9029	; 0x2345
+[^<]*<thumb\+0x28> f7f3 a456 	udf.w	#13398	; 0x3456
+[^<]*<thumb\+0x2c> f7f5 a678 	udf.w	#22136	; 0x5678
Thomas Preudhomme Oct. 26, 2018, 9:42 p.m. | #3
Yes indeed, the encoding already gives the size of the instruction
anyway. LGTM but as I said, you'll need someone else to approve.
Thanks.

Best regards,

Thomas
On Fri, 26 Oct 2018 at 15:15, Andre Vieira (lists)
<Andre.SimoesDiasVieira@arm.com> wrote:
>

> Hi Thomas,

>

> Thanks for the review.  I changed the test file and used the same as I

> did in another test, '[^<]*<', that way it also consumes the address

> which is irrelevant to this test.

>

> Cheers,

> Andre

> On 26/10/18 11:24, Thomas Preudhomme wrote:

> > Hi Andre,

> >

> > I'm not a maintainer but I have one comment nonetheless: I'd suggest

> > using the 0+ notation to absorb all the 0s in the address prefix to be

> > consistent with the rest of the testsuite (see eg.

> > gas/testsuite/gas/arm/arch6zk.d as a random example).

> >

> > Looks good to me otherwise. Best regards,

> >

> > Thomas

> > On Fri, 19 Oct 2018 at 16:00, Andre Vieira (lists)

> > <Andre.SimoesDiasVieira@arm.com> wrote:

> >>

> >> Hi,

> >>

> >> This patch fixes a testism.

> >> The old test never checked the objdump output since the 'error-output'

> >> directive

> >> would exit and thus never run objdump.  When this test was changed to

> >> adhere to

> >> use the new warning_output we started to run objdump.  The expected objdump

> >> output was old and had bitrotten, this fixes the layout, as the

> >> "disassembly"

> >> itself did not change.

> >>

> >> Is this OK for trunk?

> >>

> >> 2018-10-19  Andre Vieira  <andre.simoesdiasvieira@arm.com>

> >>

> >>         * testsuite/gas/arm/udf.d: Update expected output.

>
Andre Vieira (lists) Oct. 30, 2018, 11:18 a.m. | #4
On 26/10/18 22:42, Thomas Preudhomme wrote:
> Yes indeed, the encoding already gives the size of the instruction

> anyway. LGTM but as I said, you'll need someone else to approve.

> Thanks.

> 

> Best regards,

> 

> Thomas

> On Fri, 26 Oct 2018 at 15:15, Andre Vieira (lists)

> <Andre.SimoesDiasVieira@arm.com> wrote:

>>

>> Hi Thomas,

>>

>> Thanks for the review.  I changed the test file and used the same as I

>> did in another test, '[^<]*<', that way it also consumes the address

>> which is irrelevant to this test.

>>

>> Cheers,

>> Andre

>> On 26/10/18 11:24, Thomas Preudhomme wrote:

>>> Hi Andre,

>>>

>>> I'm not a maintainer but I have one comment nonetheless: I'd suggest

>>> using the 0+ notation to absorb all the 0s in the address prefix to be

>>> consistent with the rest of the testsuite (see eg.

>>> gas/testsuite/gas/arm/arch6zk.d as a random example).

>>>

>>> Looks good to me otherwise. Best regards,

>>>

>>> Thomas

>>> On Fri, 19 Oct 2018 at 16:00, Andre Vieira (lists)

>>> <Andre.SimoesDiasVieira@arm.com> wrote:

>>>>

>>>> Hi,

>>>>

>>>> This patch fixes a testism.

>>>> The old test never checked the objdump output since the 'error-output'

>>>> directive

>>>> would exit and thus never run objdump.  When this test was changed to

>>>> adhere to

>>>> use the new warning_output we started to run objdump.  The expected objdump

>>>> output was old and had bitrotten, this fixes the layout, as the

>>>> "disassembly"

>>>> itself did not change.

>>>>

>>>> Is this OK for trunk?

>>>>

>>>> 2018-10-19  Andre Vieira  <andre.simoesdiasvieira@arm.com>

>>>>

>>>>         * testsuite/gas/arm/udf.d: Update expected output.

>>

Ping
Ramana Radhakrishnan Oct. 30, 2018, 3:31 p.m. | #5
On Fri, Oct 26, 2018 at 3:16 PM Andre Vieira (lists)
<Andre.SimoesDiasVieira@arm.com> wrote:
>

> Hi Thomas,

>

> Thanks for the review.  I changed the test file and used the same as I

> did in another test, '[^<]*<', that way it also consumes the address

> which is irrelevant to this test.


OK.

Ramana
>

> Cheers,

> Andre

> On 26/10/18 11:24, Thomas Preudhomme wrote:

> > Hi Andre,

> >

> > I'm not a maintainer but I have one comment nonetheless: I'd suggest

> > using the 0+ notation to absorb all the 0s in the address prefix to be

> > consistent with the rest of the testsuite (see eg.

> > gas/testsuite/gas/arm/arch6zk.d as a random example).

> >

> > Looks good to me otherwise. Best regards,

> >

> > Thomas

> > On Fri, 19 Oct 2018 at 16:00, Andre Vieira (lists)

> > <Andre.SimoesDiasVieira@arm.com> wrote:

> >>

> >> Hi,

> >>

> >> This patch fixes a testism.

> >> The old test never checked the objdump output since the 'error-output'

> >> directive

> >> would exit and thus never run objdump.  When this test was changed to

> >> adhere to

> >> use the new warning_output we started to run objdump.  The expected objdump

> >> output was old and had bitrotten, this fixes the layout, as the

> >> "disassembly"

> >> itself did not change.

> >>

> >> Is this OK for trunk?

> >>

> >> 2018-10-19  Andre Vieira  <andre.simoesdiasvieira@arm.com>

> >>

> >>         * testsuite/gas/arm/udf.d: Update expected output.

>

Patch

diff --git a/gas/testsuite/gas/arm/udf.d b/gas/testsuite/gas/arm/udf.d
index c8777a9f052e71ad679a27c64cf666afa1dcf38f..598789e095a5722e03ca38185e9ed21f0332b7f1 100644
--- a/gas/testsuite/gas/arm/udf.d
+++ b/gas/testsuite/gas/arm/udf.d
@@ -6,26 +6,22 @@ 
 .*: +file format .*arm.*
 
 Disassembly of section \.text:
-
-0+0 <arm>:
-\s*0:\s+e7f000f0\s+udf	#0
-\s*4:\s+e7fabcfd\s+udf	#43981	; 0xabcd
-
-0+0 <thumb>:
-\s*8:\s+deab\s+udf	#171	; 0xab
-\s*a:\s+decd\s+udf	#205	; 0xcd
-\s*c:\s+de00\s+udf	#0
-\s*e:\s+46c0\s+nop.*
-\s*10:\s+f7f0 a000\s+udf\.w	#0
-\s*14:\s+f7f1 a234\s+udf\.w	#4660	; 0x1234
-\s*18:\s+f7fc acdd\s+udf\.w	#52445	; 0xccdd
-\s*1c:\s+bf08\s+it	eq
-\s*1e:\s+de12\s+udfeq	#18
-\s*20:\s+de23\s+udf	#35	; 0x23
-\s*22:\s+de34\s+udf	#52	; 0x34
-\s*24:\s+de56\s+udf	#86	; 0x56
-\s*26:\s+bf18\s+it	ne
-\s*28:\s+f7f1 a234\s+udfne\.w	#4660	; 0x1234
-\s*2c:\s+f7f2 a345\s+udf\.w	#9029	; 0x2345
-\s*30:\s+f7f3 a456\s+udf\.w	#13398	; 0x3456
-\s*34:\s+f7f5 a678\s+udf\.w	#22136	; 0x5678
+00000000 <arm> e7f000f0 	udf	#0
+00000004 <arm\+0x4> e7fabcfd 	udf	#43981	; 0xabcd
+00000008 <thumb> deab      	udf	#171	; 0xab
+0000000a <thumb\+0x2> decd      	udf	#205	; 0xcd
+0000000c <thumb\+0x4> de00      	udf	#0
+0000000e <thumb\+0x6> bf00      	nop
+00000010 <thumb\+0x8> f7f0 a000 	udf.w	#0
+00000014 <thumb\+0xc> f7f1 a234 	udf.w	#4660	; 0x1234
+00000018 <thumb\+0x10> f7fc acdd 	udf.w	#52445	; 0xccdd
+0000001c <thumb\+0x14> bf08      	it	eq
+0000001e <thumb\+0x16> de12      	udfeq	#18
+00000020 <thumb\+0x18> de23      	udf	#35	; 0x23
+00000022 <thumb\+0x1a> de34      	udf	#52	; 0x34
+00000024 <thumb\+0x1c> de56      	udf	#86	; 0x56
+00000026 <thumb\+0x1e> bf18      	it	ne
+00000028 <thumb\+0x20> f7f1 a234 	udfne.w	#4660	; 0x1234
+0000002c <thumb\+0x24> f7f2 a345 	udf.w	#9029	; 0x2345
+00000030 <thumb\+0x28> f7f3 a456 	udf.w	#13398	; 0x3456
+00000034 <thumb\+0x2c> f7f5 a678 	udf.w	#22136	; 0x5678