[PATCH] GAS/MIPS: Fix testcase module-defer-warn2 for r2+ triples

[PATCH] GAS/MIPS: Fix testcase module-defer-warn2 for r2+ triples

[YunQiang Su]

When gas is configured with --target=mipsisa32r2el-elf, module-defer-warn2
will fail:

/binutils-gdb/gas/testsuite/gas/mips/module-defer-warn2.s: Assembler messages:
/binutils-gdb/gas/testsuite/gas/mips/module-defer-warn2.s:2: Error: `gp=64' used with a 32-bit processor
extra regexps in /binutils-gdb/gas/testsuite/gas/mips/module-defer-warn2.l starting with "^.*:2: .*: `fp=64' used with a 32-bit.*$"
EOF from dump.out
FAIL: mips module-defer-warn2

The reason is that fp64 is allowed for mips32r2 and onward, so
the error message `Error: `fp=64' used with a 32-bit fpu` won't emit.
---
 gas/testsuite/gas/mips/module-defer-warn2.l | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gas/testsuite/gas/mips/module-defer-warn2.l b/gas/testsuite/gas/mips/module-defer-warn2.l
index bd37c299434..95ceb647b94 100644
--- a/gas/testsuite/gas/mips/module-defer-warn2.l
+++ b/gas/testsuite/gas/mips/module-defer-warn2.l
@@ -1,3 +1,3 @@
 .*: Assembler messages:
 .*:2: Error: `gp=64' used with a 32-bit.*
-.*:2: .*: `fp=64' used with a 32-bit.*
+#?.*:2: .*: `fp=64' used with a 32-bit.*
-- 
2.39.2

Re: [PATCH] GAS/MIPS: Fix testcase module-defer-warn2 for r2+ triples

[Maciej W. Rozycki]

On Mon, 28 Aug 2023, YunQiang Su wrote:

> When gas is configured with --target=mipsisa32r2el-elf, module-defer-warn2
> will fail:
> 
> /binutils-gdb/gas/testsuite/gas/mips/module-defer-warn2.s: Assembler messages:
> /binutils-gdb/gas/testsuite/gas/mips/module-defer-warn2.s:2: Error: `gp=64' used with a 32-bit processor
> extra regexps in /binutils-gdb/gas/testsuite/gas/mips/module-defer-warn2.l starting with "^.*:2: .*: `fp=64' used with a 32-bit.*$"
> EOF from dump.out
> FAIL: mips module-defer-warn2
> 
> The reason is that fp64 is allowed for mips32r2 and onward, so
> the error message `Error: `fp=64' used with a 32-bit fpu` won't emit.

 It has to be a set of separate tests then, because the very purpose of 
the test is to verify all the error messages required are there.  Making 
any optional will defeat the purpose of the test and weaken coverage.

 It seems to me that the best course of action will be converting the test 
to the .d format first, which gives more control, and then split it into 
two, using #skip/#noskip tags as appropriate to select the right one for 
the respective targets.

  Maciej

[PATCH v2] GAS/MIPS: Fix testcase module-defer-warn2 for r2+ triples

[YunQiang Su]

When gas is configured with --target=mipsisa32r2el-elf,  module-defer-warn2
will fail:

/binutils-gdb/gas/testsuite/gas/mips/module-defer-warn2.s: Assembler messages:
/binutils-gdb/gas/testsuite/gas/mips/module-defer-warn2.s:2: Error: `gp=64' used with a 32-bit processor
extra regexps in /binutils-gdb/gas/testsuite/gas/mips/module-defer-warn2.l starting with "^.*:2: .*: `fp=64' used with a 32-bit.*$"
EOF from dump.out
FAIL: mips module-defer-warn2

The reason is that fp64 is allowed for mips32r2 and onward,  so
the error message `Error: `fp=64' used with a 32-bit fpu` won't emit.

Let's convert this testcase to `.d' format,  and split it to
   module-defer-warn2-r2
   module-defer-warn2-prer2,
and use `skip/target` tags to select the right triples.
---
 gas/testsuite/gas/mips/mips.exp                              | 3 ++-
 gas/testsuite/gas/mips/module-defer-warn2-prer2.d            | 5 +++++
 .../{module-defer-warn2.l => module-defer-warn2-prer2.l}     | 0
 gas/testsuite/gas/mips/module-defer-warn2-r2.d               | 5 +++++
 gas/testsuite/gas/mips/module-defer-warn2-r2.l               | 2 ++
 5 files changed, 14 insertions(+), 1 deletion(-)
 create mode 100644 gas/testsuite/gas/mips/module-defer-warn2-prer2.d
 rename gas/testsuite/gas/mips/{module-defer-warn2.l => module-defer-warn2-prer2.l} (100%)
 create mode 100644 gas/testsuite/gas/mips/module-defer-warn2-r2.d
 create mode 100644 gas/testsuite/gas/mips/module-defer-warn2-r2.l

diff --git a/gas/testsuite/gas/mips/mips.exp b/gas/testsuite/gas/mips/mips.exp
index 91cf8b11077..6e2b41d9e59 100644
--- a/gas/testsuite/gas/mips/mips.exp
+++ b/gas/testsuite/gas/mips/mips.exp
@@ -2059,7 +2059,8 @@ if { [istarget mips*-*-vxworks*] } {
 
     run_dump_test "module-override"
     run_dump_test "module-defer-warn1"
-    run_list_test "module-defer-warn2" "-32"
+    run_dump_test "module-defer-warn2-prer2"
+    run_dump_test "module-defer-warn2-r2"
 
     foreach testopt [list -mfp32 -mfpxx -mfp64 "-mfp64-noodd" \
 			  -msingle-float -msoft-float] {
diff --git a/gas/testsuite/gas/mips/module-defer-warn2-prer2.d b/gas/testsuite/gas/mips/module-defer-warn2-prer2.d
new file mode 100644
index 00000000000..9b2b3c4b51a
--- /dev/null
+++ b/gas/testsuite/gas/mips/module-defer-warn2-prer2.d
@@ -0,0 +1,5 @@
+#name: .module deferred warnings 2 (pre-R2)
+#source: module-defer-warn2.s
+#as: -32
+#skip: mipsisa32r?* mipsisa64r?*
+#error_output: module-defer-warn2-prer2.l
diff --git a/gas/testsuite/gas/mips/module-defer-warn2.l b/gas/testsuite/gas/mips/module-defer-warn2-prer2.l
similarity index 100%
rename from gas/testsuite/gas/mips/module-defer-warn2.l
rename to gas/testsuite/gas/mips/module-defer-warn2-prer2.l
diff --git a/gas/testsuite/gas/mips/module-defer-warn2-r2.d b/gas/testsuite/gas/mips/module-defer-warn2-r2.d
new file mode 100644
index 00000000000..07504379d3b
--- /dev/null
+++ b/gas/testsuite/gas/mips/module-defer-warn2-r2.d
@@ -0,0 +1,5 @@
+#name: .module deferred warnings 2 (R2+)
+#source: module-defer-warn2.s
+#as: -32
+#target: mipsisa32r?* mipsisa64r?*
+#error_output: module-defer-warn2-r2.l
diff --git a/gas/testsuite/gas/mips/module-defer-warn2-r2.l b/gas/testsuite/gas/mips/module-defer-warn2-r2.l
new file mode 100644
index 00000000000..5f22ef4d413
--- /dev/null
+++ b/gas/testsuite/gas/mips/module-defer-warn2-r2.l
@@ -0,0 +1,2 @@
+.*: Assembler messages:
+.*:2: Error: `gp=64' used with a 32-bit.*
-- 
2.39.2

Re: [PATCH v2] GAS/MIPS: Fix testcase module-defer-warn2 for r2+ triples

[Maciej W. Rozycki]

On Tue, 26 Sep 2023, YunQiang Su wrote:

> Let's convert this testcase to `.d' format,  and split it to
>    module-defer-warn2-r2
>    module-defer-warn2-prer2,
> and use `skip/target` tags to select the right triples.

 As I told you with v1:

>  It seems to me that the best course of action will be converting the test 
> to the .d format first, which gives more control, and then split it into 
> two, using #skip/#noskip tags as appropriate to select the right one for 
> the respective targets.

-- so it has to be a series of two patches:

* 1/2 to convert the test case to the .d format (keeping the semantics
  the same),

* 2/2 to split it into two, using #skip/#noskip tags.

NB why did you use #target rather than #noskip?

 Also please start a new thread when submitting updated patches rather 
than replying to an old one.

  Maciej

Re: [PATCH v2] GAS/MIPS: Fix testcase module-defer-warn2 for r2+ triples

[YunQiang Su]

Maciej W. Rozycki <macro@orcam.me.uk> 于2023年9月29日周五 07:46写道：
>
> On Tue, 26 Sep 2023, YunQiang Su wrote:
>
> > Let's convert this testcase to `.d' format,  and split it to
> >    module-defer-warn2-r2
> >    module-defer-warn2-prer2,
> > and use `skip/target` tags to select the right triples.
>
>  As I told you with v1:
>
> >  It seems to me that the best course of action will be converting the test
> > to the .d format first, which gives more control, and then split it into
> > two, using #skip/#noskip tags as appropriate to select the right one for
> > the respective targets.
>
> -- so it has to be a series of two patches:
>
> * 1/2 to convert the test case to the .d format (keeping the semantics
>   the same),
>
> * 2/2 to split it into two, using #skip/#noskip tags.
>

I will do so.

> NB why did you use #target rather than #noskip?
>

Sorry. I misunderstand the meaning of "#noskip".

>  Also please start a new thread when submitting updated patches rather
> than replying to an old one.
>

I will do so.

>   Maciej



-- 
YunQiang Su

Re: [PATCH v2] GAS/MIPS: Fix testcase module-defer-warn2 for r2+ triples

[Maciej W. Rozycki]

On Fri, 29 Sep 2023, YunQiang Su wrote:

> >  As I told you with v1:
> >
> > >  It seems to me that the best course of action will be converting the test
> > > to the .d format first, which gives more control, and then split it into
> > > two, using #skip/#noskip tags as appropriate to select the right one for
> > > the respective targets.
> >
> > -- so it has to be a series of two patches:
> >
> > * 1/2 to convert the test case to the .d format (keeping the semantics
> >   the same),
> >
> > * 2/2 to split it into two, using #skip/#noskip tags.
> >
> 
> I will do so.

 As always one self-contained change per patch please.  Switching to the 
.d format is one change and fixing R2+ support is another.  So it has to 
be two separate changes.

  Maciej

Re: [PATCH v2] GAS/MIPS: Fix testcase module-defer-warn2 for r2+ triples

[YunQiang Su]

在 2023/9/29 22:18, Maciej W. Rozycki 写道:
> On Fri, 29 Sep 2023, YunQiang Su wrote:
>
>>>   As I told you with v1:
>>>
>>>>   It seems to me that the best course of action will be converting the test
>>>> to the .d format first, which gives more control, and then split it into
>>>> two, using #skip/#noskip tags as appropriate to select the right one for
>>>> the respective targets.
>>> -- so it has to be a series of two patches:
>>>
>>> * 1/2 to convert the test case to the .d format (keeping the semantics
>>>    the same),
>>>
>>> * 2/2 to split it into two, using #skip/#noskip tags.
>>>
>> I will do so.
>   As always one self-contained change per patch please.  Switching to the
> .d format is one change and fixing R2+ support is another.  So it has to
> be two separate changes.


To be make thing more clear, and I am worrying that, the patch can be 
spilt to 3 even:

1. switch to .d format

2. add "#skip" tag

3. add new r2 tests.


So, if we'd like to split to 2 patches, should they are {{1,2}, 3} or 
{1, {2, 3}}, or should we

split it to 3 ones?


>    Maciej

Re: [PATCH v2] GAS/MIPS: Fix testcase module-defer-warn2 for r2+ triples

[Maciej W. Rozycki]

On Fri, 29 Sep 2023, YunQiang Su wrote:

> >   As always one self-contained change per patch please.  Switching to the
> > .d format is one change and fixing R2+ support is another.  So it has to
> > be two separate changes.
> 
> 
> To be make thing more clear, and I am worrying that, the patch can be spilt to
> 3 even:
> 
> 1. switch to .d format
> 
> 2. add "#skip" tag
> 
> 3. add new r2 tests.
> 
> 
> So, if we'd like to split to 2 patches, should they are {{1,2}, 3} or {1, {2,
> 3}}, or should we
> 
> split it to 3 ones?

 Thanks for raising this point, but I think there is no need to add 
"#skip" in a separate step for two reasons:

1. The switch to the .d format preserves the current situation, that is
   the test working correctly for R1- and not working correctly for R2+.  
   There's no need to add "#skip" at this stage.

2. A fix to make the test for R2+ is one functionally self-contained
   change and as documented "#skip"/"#noskip" tags are supposed to be used 
   in pairs (or groups of multiple as required in ternary or wider cases) 
   to cover variants of one specific test, so having "#skip" with no 
   corresponding "#noskip" would in fact be incorrect.  So it has to be 
   made in a single change.

I hope I made myself clear here.

  Maciej