[PATCH] MIPS/GAS: Use addiu instead of addi in test elf-rel

[PATCH] MIPS/GAS: Use addiu instead of addi in test elf-rel

[YunQiang Su]

ADDI has been removed in MIPSr6, and thus these cases fail.
For all releases, ADDIU is recommended.

The only difference between ADDI and ADDIU, is that ADDI will trap
if overflow, and ADDIU won't.

This patch can fix a test failure on MIPSr6 default triples:
    MIPS ELF reloc
---
 gas/testsuite/gas/mips/elf-rel.d   | 12 +++++-----
 gas/testsuite/gas/mips/elf-rel.s   | 36 +++++++++++++++---------------
 gas/testsuite/gas/mips/elfel-rel.d | 12 +++++-----
 3 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/gas/testsuite/gas/mips/elf-rel.d b/gas/testsuite/gas/mips/elf-rel.d
index bb7077eb771..27d3d8848d4 100644
--- a/gas/testsuite/gas/mips/elf-rel.d
+++ b/gas/testsuite/gas/mips/elf-rel.d
@@ -48,12 +48,12 @@ OFFSET +TYPE +VALUE
 
 Contents of section \.text:
  0000 3c010000 3c010000 3c010001 3c010001  .*
- 0010 3c010000 3c010001 20210018 2021001c  .*
- 0020 20210018 2021001c 20218018 2021fffc  .*
+ 0010 3c010000 3c010001 24210018 2421001c  .*
+ 0020 24210018 2421001c 24218018 2421fffc  .*
  0030 3c010001 3c010001 3c010002 3c010002  .*
- 0040 3c010001 3c010001 2021bffe 2021c002  .*
- 0050 2021bffe 2021c002 20213ffe 2021bffa  .*
+ 0040 3c010001 3c010001 2421bffe 2421c002  .*
+ 0050 2421bffe 2421c002 24213ffe 2421bffa  .*
  0060 3c010001 3c010001 3c010002 3c010002  .*
- 0070 3c010001 3c010001 2021bffe 2021c002  .*
- 0080 2021bffe 2021c002 20213ffe 2021bffa  .*
+ 0070 3c010001 3c010001 2421bffe 2421c002  .*
+ 0080 2421bffe 2421c002 24213ffe 2421bffa  .*
 #pass
diff --git a/gas/testsuite/gas/mips/elf-rel.s b/gas/testsuite/gas/mips/elf-rel.s
index 873bc5fd86a..06c67a659bf 100644
--- a/gas/testsuite/gas/mips/elf-rel.s
+++ b/gas/testsuite/gas/mips/elf-rel.s
@@ -12,12 +12,12 @@ l2	= l0+49150
 	lui	$at,%hi(l0-4)
 	lui	$at,%hi(l1+0x8000)
 l1:		
-	addi	$at,$at,%lo(l1)
-	addi	$at,$at,%lo(l1+0x10004)
-	addi	$at,$at,%lo(l1+0x10000)
-	addi	$at,$at,%lo(l1+4)
-	addi	$at,$at,%lo(l1+0x8000)
-	addi	$at,$at,%lo(l0-4)
+	addiu	$at,$at,%lo(l1)
+	addiu	$at,$at,%lo(l1+0x10004)
+	addiu	$at,$at,%lo(l1+0x10000)
+	addiu	$at,$at,%lo(l1+4)
+	addiu	$at,$at,%lo(l1+0x8000)
+	addiu	$at,$at,%lo(l0-4)
 
 	lui	$at,%hi(l2)
 	lui	$at,%hi(l2+4)
@@ -25,12 +25,12 @@ l1:
 	lui	$at,%hi(l2+0x10004)
 	lui	$at,%hi(l2-4)
 	lui	$at,%hi(l2+0x8000)
-	addi	$at,$at,%lo(l2)
-	addi	$at,$at,%lo(l2+4)
-	addi	$at,$at,%lo(l2+0x10000)
-	addi	$at,$at,%lo(l2+0x10004)
-	addi	$at,$at,%lo(l2+0x8000)
-	addi	$at,$at,%lo(l2-4)
+	addiu	$at,$at,%lo(l2)
+	addiu	$at,$at,%lo(l2+4)
+	addiu	$at,$at,%lo(l2+0x10000)
+	addiu	$at,$at,%lo(l2+0x10004)
+	addiu	$at,$at,%lo(l2+0x8000)
+	addiu	$at,$at,%lo(l2-4)
 
 	lui	$at,%hi((l2))
 	lui	$at,%hi(((l2+4)))
@@ -38,9 +38,9 @@ l1:
 	lui	$at,%hi(((((l2+0x10004)))))
 	lui	$at,%hi((((((l2-4))))))
 	lui	$at,%hi(((((((l2+0x8000)))))))
-	addi	$at,$at,%lo((l2))
-	addi	$at,$at,%lo(((l2+4)))
-	addi	$at,$at,%lo((((l2+0x10000))))
-	addi	$at,$at,%lo(((((l2+0x10004)))))
-	addi	$at,$at,%lo((((((l2+0x8000))))))
-	addi	$at,$at,%lo(((((((l2-4)))))))
+	addiu	$at,$at,%lo((l2))
+	addiu	$at,$at,%lo(((l2+4)))
+	addiu	$at,$at,%lo((((l2+0x10000))))
+	addiu	$at,$at,%lo(((((l2+0x10004)))))
+	addiu	$at,$at,%lo((((((l2+0x8000))))))
+	addiu	$at,$at,%lo(((((((l2-4)))))))
diff --git a/gas/testsuite/gas/mips/elfel-rel.d b/gas/testsuite/gas/mips/elfel-rel.d
index 7a9a3b92bfb..11fc7ad2157 100644
--- a/gas/testsuite/gas/mips/elfel-rel.d
+++ b/gas/testsuite/gas/mips/elfel-rel.d
@@ -49,12 +49,12 @@ OFFSET +TYPE +VALUE
 
 Contents of section \.text:
  0000 0000013c 0000013c 0100013c 0100013c  .*
- 0010 0000013c 0100013c 18002120 1c002120  .*
- 0020 18002120 1c002120 18802120 fcff2120  .*
+ 0010 0000013c 0100013c 18002124 1c002124  .*
+ 0020 18002124 1c002124 18802124 fcff2124  .*
  0030 0100013c 0100013c 0200013c 0200013c  .*
- 0040 0100013c 0100013c febf2120 02c02120  .*
- 0050 febf2120 02c02120 fe3f2120 fabf2120  .*
+ 0040 0100013c 0100013c febf2124 02c02124  .*
+ 0050 febf2124 02c02124 fe3f2124 fabf2124  .*
  0060 0100013c 0100013c 0200013c 0200013c  .*
- 0070 0100013c 0100013c febf2120 02c02120  .*
- 0080 febf2120 02c02120 fe3f2120 fabf2120  .*
+ 0070 0100013c 0100013c febf2124 02c02124  .*
+ 0080 febf2124 02c02124 fe3f2124 fabf2124  .*
 #pass
-- 
2.39.2

Re: [PATCH] MIPS/GAS: Use addiu instead of addi in test elf-rel

[Nick Clifton]

Hi YunQiang Su,

> ADDI has been removed in MIPSr6, and thus these cases fail.
> For all releases, ADDIU is recommended.
> 
> The only difference between ADDI and ADDIU, is that ADDI will trap
> if overflow, and ADDIU won't.
> 
> This patch can fix a test failure on MIPSr6 default triples:
>      MIPS ELF reloc
> ---
>   gas/testsuite/gas/mips/elf-rel.d   | 12 +++++-----
>   gas/testsuite/gas/mips/elf-rel.s   | 36 +++++++++++++++---------------
>   gas/testsuite/gas/mips/elfel-rel.d | 12 +++++-----

Approved and applied.

Cheers
   Nick

Re: [PATCH] MIPS/GAS: Use addiu instead of addi in test elf-rel

[Maciej W. Rozycki]

On Thu, 23 Nov 2023, Nick Clifton wrote:

> > ADDI has been removed in MIPSr6, and thus these cases fail.
> > For all releases, ADDIU is recommended.
> > 
> > The only difference between ADDI and ADDIU, is that ADDI will trap
> > if overflow, and ADDIU won't.
> > 
> > This patch can fix a test failure on MIPSr6 default triples:
> >      MIPS ELF reloc
> > ---
> >   gas/testsuite/gas/mips/elf-rel.d   | 12 +++++-----
> >   gas/testsuite/gas/mips/elf-rel.s   | 36 +++++++++++++++---------------
> >   gas/testsuite/gas/mips/elfel-rel.d | 12 +++++-----
> 
> Approved and applied.

 The 64-bit MIPS psABI actually specified ADDI rather than ADDIU for 
certain address load sequences and <sys/asm.h> (or Linux <asm/asm.h>) 
macros used to use it.  We abandoned this practice at one point, but I 
find to deliberately remove verification for this instruction rather 
gratuitous.  This should have been done in a better way, still without 
sacrificing MIPSr6 coverage.

 In any case the change description shouldn't have been dropped from the 
commit, which I can see has been correctly prepared for applying via `git 
am'.  Right now it presents itself in the repo as if it's been done with 
no justification whatsoever, and certainly whoever comes across it in a 
few year's time will scratch their head about it.

  Maciej

Re: [PATCH] MIPS/GAS: Use addiu instead of addi in test elf-rel

[Nick Clifton]

Hi Maciej,

>>>    gas/testsuite/gas/mips/elf-rel.d   | 12 +++++-----
>>>    gas/testsuite/gas/mips/elf-rel.s   | 36 +++++++++++++++---------------
>>>    gas/testsuite/gas/mips/elfel-rel.d | 12 +++++-----
>>
>> Approved and applied.
> 
>   The 64-bit MIPS psABI actually specified ADDI rather than ADDIU for
> certain address load sequences and <sys/asm.h> (or Linux <asm/asm.h>)
> macros used to use it.  We abandoned this practice at one point, but I
> find to deliberately remove verification for this instruction rather
> gratuitous.  This should have been done in a better way, still without
> sacrificing MIPSr6 coverage.

By having an r6 version of the test perhaps ?

>   In any case the change description shouldn't have been dropped from the
> commit, which I can see has been correctly prepared for applying via `git
> am'.  Right now it presents itself in the repo as if it's been done with
> no justification whatsoever, and certainly whoever comes across it in a
> few year's time will scratch their head about it.

This is my fault.  I have no idea how to use 'git am', instead I just apply
patches by hand.  If you do not mind, please could you fix this for me so
that the correct description is there ?

Thanks.

Cheers
   Nick

Re: [PATCH] MIPS/GAS: Use addiu instead of addi in test elf-rel

[YunQiang Su]

> This is my fault.  I have no idea how to use 'git am', instead I just apply
> patches by hand.  If you do not mind, please could you fix this for me so
> that the correct description is there ?
>

For hand work, you can just download the original mailbox file may be
some like "*.eml", then
    git am --ignore-date xx.eml

-- 
YunQiang Su

Re: [PATCH] MIPS/GAS: Use addiu instead of addi in test elf-rel

[Maciej W. Rozycki]

Hi Nick,

> >   The 64-bit MIPS psABI actually specified ADDI rather than ADDIU for
> > certain address load sequences and <sys/asm.h> (or Linux <asm/asm.h>)
> > macros used to use it.  We abandoned this practice at one point, but I
> > find to deliberately remove verification for this instruction rather
> > gratuitous.  This should have been done in a better way, still without
> > sacrificing MIPSr6 coverage.
> 
> By having an r6 version of the test perhaps ?

 Yes.  Besides, I think R6 should actually work too, though with different 
output, by interpreting ADDI as a macro and expanding it to an equivalent 
LI/ADD instruction pair (with any relocation applied to the LI operation).  
That has been the principle of the MIPS assembly language dialect, and it 
is how pre-R6 to R6 transition was supposed to address backwards source 
compatibility and minimise impact on existing software (similarly to the 
transition from the conventional MIPS ISA to the original microMIPS ISA).

> >   In any case the change description shouldn't have been dropped from the
> > commit, which I can see has been correctly prepared for applying via `git
> > am'.  Right now it presents itself in the repo as if it's been done with
> > no justification whatsoever, and certainly whoever comes across it in a
> > few year's time will scratch their head about it.
> 
> This is my fault.  I have no idea how to use 'git am', instead I just apply
> patches by hand.  If you do not mind, please could you fix this for me so
> that the correct description is there ?

 Unfortunately a commit description, once the commit has been pushed, can 
only be retrofitted by means of a rebase and we forbid (non-fast-forward) 
rebases by policy, as doing so would disturb people's trees out there.

 Applying patches by hand must be a pain, I sympathise.  My e-mail client 
has a "pipe" command, which feeds the message selected, usually one that's 
currently shown, though several messages can be fed all at once too, in 
its cooked form (i.e. as output to the terminal, rather than the raw form, 
QP or Base64, it has been encoded for transport) to the standard input of 
an arbitrary shell pipeline.

 So I pipe a message to be applied to: `cd /path/to/repository && git am'.  
I can then push the contents of /path/to/repository to their origin right 
away (I usually use commands like `git show' beforehand to verify the 
result of `git am' meets my expectations; at this point I can still make 
final tweaks with `git commit --amend').

 If your e-mail client has no such feature, then I do hope it can at least 
export a message complete with its headers to a text file, which you can 
feed similarly: `cat message.txt | (cd /path/to/repository && git am)' or 
suchlike.

 I do recommend using `git am' whenever possible, as it will accurately
record commit authorship according to the `From:' message header or any 
override given in the first line of the message body, it will transfer the 
`Subject:' header to the change heading, and it will make the message body 
(sans any `From:' override) the change description.  And it will of course 
apply the patch included as well.  All of which automatically, making it 
easier to avoid human mistakes.

 I do encourage reading the git-am(1) man page for further details, and if 
you still have any questions afterwards, then feel free to ask me directly 
and I'll strive to answer them.

  Maciej