diff mbox series

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

Message ID 20230926112035.2692284-1-yunqiang.su@cipunited.com
State Superseded
Headers
Series [v2] GAS/MIPS: Fix testcase module-defer-warn2 for r2+ triples |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 fail Testing failed
linaro-tcwg-bot/tcwg_binutils_check--master-arm fail Testing failed
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Testing passed

Commit Message

YunQiang Su Sept. 26, 2023, 11:20 a.m. UTC 
  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

Comments

Maciej W. Rozycki Sept. 28, 2023, 11:46 p.m. UTC | #1 
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
YunQiang Su Sept. 29, 2023, 1:41 p.m. UTC | #2 
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
Maciej W. Rozycki Sept. 29, 2023, 2:18 p.m. UTC | #3 
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
YunQiang Su Sept. 29, 2023, 2:23 p.m. UTC | #4 
在 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
Maciej W. Rozycki Sept. 29, 2023, 2:32 p.m. UTC | #5 
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
diff mbox series

Patch

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.*