[1/3] RISC-V: prefer SLT{,U} aliases for SLTI{,U}

Message ID 95936261-d824-9128-1be9-ba7dfe12b042@suse.com
State Accepted
Headers
Series RISC-V: further alias insn handling adjustments |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Jan Beulich Jan. 13, 2023, 10:19 a.m. UTC
  While not spelled out by the spec, gas has been supporting these aliases
virtually forever. Yet they were unused by the disassembler because of
sitting later in the table. Move the non-aliases down.
  

Comments

Maciej W. Rozycki Jan. 15, 2023, 4:35 a.m. UTC | #1
On Fri, 13 Jan 2023, Jan Beulich via Binutils wrote:

> While not spelled out by the spec, gas has been supporting these aliases
> virtually forever. Yet they were unused by the disassembler because of
> sitting later in the table. Move the non-aliases down.

 I think this is going backwards.  Aliases are used in disassembly to 
improve readability, e.g. to show `nop', rather than `c.addi zero,0' which 
would make anyone scratch their head, at least initially.

 In this case there's no improvement, but obfuscation, as you're losing 
the clear distinction between the register and the immediate instructions 
and one will have to examine the operands to spot the difference.

 The alternative mnemonic forms for immediate machine instructions are 
there (NB pinched from the MIPS assembly dialect) to make it easier for 
people to write handcoded assembly, especially where macros are involved, 
either GAS or C preprocessor ones, and not for disassembly.  Conceptually 
they're assembly macros that expand to a single instruction rather than 
aliases.  IOW it's not a bug that those are not considered aliases for 
disassembly.

 Likewise with the remaining patches in this series.

  Maciej
  
Jan Beulich Jan. 16, 2023, 7:46 a.m. UTC | #2
On 15.01.2023 05:35, Maciej W. Rozycki wrote:
> On Fri, 13 Jan 2023, Jan Beulich via Binutils wrote:
> 
>> While not spelled out by the spec, gas has been supporting these aliases
>> virtually forever. Yet they were unused by the disassembler because of
>> sitting later in the table. Move the non-aliases down.
> 
>  I think this is going backwards.  Aliases are used in disassembly to 
> improve readability, e.g. to show `nop', rather than `c.addi zero,0' which 
> would make anyone scratch their head, at least initially.
> 
>  In this case there's no improvement, but obfuscation, as you're losing 
> the clear distinction between the register and the immediate instructions 
> and one will have to examine the operands to spot the difference.

I can see this as a way to look at things, but then there need to be
changes in the other direction (after all patches 1 and 3 here merely
follow existing practice). Plus, perhaps more importantly, if you
don't strictly prefer aliases over "real" insns, then first of all it
needs establishing (and writing down) where to draw the boundary.

My view is that the "i" in the name is needlessly distinguishing the
mnemonics from their non-immediate counterparts (i.e. I view
"improvement" vs "obfuscation the other way around"). I'm surely
biased from architectures like x86, IA-64, or Arm, where mnemonic
names don't try to duplicate what's expressed by operands. Much like
you ...

>  The alternative mnemonic forms for immediate machine instructions are 
> there (NB pinched from the MIPS assembly dialect) to make it easier for 
> people to write handcoded assembly, especially where macros are involved, 
> either GAS or C preprocessor ones, and not for disassembly.  Conceptually 
> they're assembly macros that expand to a single instruction rather than 
> aliases.  IOW it's not a bug that those are not considered aliases for 
> disassembly.

... may be biased by your MIPS experience.

>  Likewise with the remaining patches in this series.

Perhaps patch 3, but patch 2 doesn't alter disassembly (and has a
different purpose).

Jan
  
Maciej W. Rozycki Jan. 25, 2023, 12:42 a.m. UTC | #3
On Mon, 16 Jan 2023, Jan Beulich wrote:

> >  I think this is going backwards.  Aliases are used in disassembly to 
> > improve readability, e.g. to show `nop', rather than `c.addi zero,0' which 
> > would make anyone scratch their head, at least initially.
> > 
> >  In this case there's no improvement, but obfuscation, as you're losing 
> > the clear distinction between the register and the immediate instructions 
> > and one will have to examine the operands to spot the difference.
> 
> I can see this as a way to look at things, but then there need to be
> changes in the other direction (after all patches 1 and 3 here merely
> follow existing practice). Plus, perhaps more importantly, if you
> don't strictly prefer aliases over "real" insns, then first of all it
> needs establishing (and writing down) where to draw the boundary.
> 
> My view is that the "i" in the name is needlessly distinguishing the
> mnemonics from their non-immediate counterparts (i.e. I view
> "improvement" vs "obfuscation the other way around"). I'm surely
> biased from architectures like x86, IA-64, or Arm, where mnemonic
> names don't try to duplicate what's expressed by operands. Much like
> you ...

 This is however what these instructions have been named in the ISA and 
the assembly dialect.  In the case of NOP, MOVE, etc. mnemonics they are 
significant assembly idioms (usually mentioned in the ISA manual) and 
there are sometimes thousands of alternative encodings that could be used 
to effect the same operation, but only the chosen canonical encoding is 
disassembled this way.

> >  The alternative mnemonic forms for immediate machine instructions are 
> > there (NB pinched from the MIPS assembly dialect) to make it easier for 
> > people to write handcoded assembly, especially where macros are involved, 
> > either GAS or C preprocessor ones, and not for disassembly.  Conceptually 
> > they're assembly macros that expand to a single instruction rather than 
> > aliases.  IOW it's not a bug that those are not considered aliases for 
> > disassembly.
> 
> ... may be biased by your MIPS experience.

 It's rather how the assembly language has been designed (FWIW the RISC-V 
ISA and assembly dialect have been largely inspired by the MIPS approach).  
NB the POWER ISA also uses different mnemonics in its assembly dialect for 
immediate ALU ("i") or indexed memory ("x") machine operations vs their 
register or displacement variants.  There are other such assembly dialects 
I suppose (IIRC Intel 8080 is like that and unlike Zilog Z80, despite the 
backwards compatibility of the ISA).

> >  Likewise with the remaining patches in this series.
> 
> Perhaps patch 3, but patch 2 doesn't alter disassembly (and has a
> different purpose).

 Ack.  FWIW I think grouping aliases together has a value, but I have no 
strong opinion here.  The implementation mandates that encodings reusing 
the same mnemonic be grouped together and that limits the ordering options 
available, i.e. you can't have all aliases at the beginning of the table 
and you can't have all the entries encoding the same machine operation 
next to each other either.

  Maciej
  
Jan Beulich Jan. 25, 2023, 3:02 p.m. UTC | #4
On 25.01.2023 01:42, Maciej W. Rozycki wrote:
> On Mon, 16 Jan 2023, Jan Beulich wrote:
> 
>>>  I think this is going backwards.  Aliases are used in disassembly to 
>>> improve readability, e.g. to show `nop', rather than `c.addi zero,0' which 
>>> would make anyone scratch their head, at least initially.
>>>
>>>  In this case there's no improvement, but obfuscation, as you're losing 
>>> the clear distinction between the register and the immediate instructions 
>>> and one will have to examine the operands to spot the difference.
>>
>> I can see this as a way to look at things, but then there need to be
>> changes in the other direction (after all patches 1 and 3 here merely
>> follow existing practice). Plus, perhaps more importantly, if you
>> don't strictly prefer aliases over "real" insns, then first of all it
>> needs establishing (and writing down) where to draw the boundary.
>>
>> My view is that the "i" in the name is needlessly distinguishing the
>> mnemonics from their non-immediate counterparts (i.e. I view
>> "improvement" vs "obfuscation the other way around"). I'm surely
>> biased from architectures like x86, IA-64, or Arm, where mnemonic
>> names don't try to duplicate what's expressed by operands. Much like
>> you ...
> 
>  This is however what these instructions have been named in the ISA and 
> the assembly dialect.  In the case of NOP, MOVE, etc. mnemonics they are 
> significant assembly idioms (usually mentioned in the ISA manual) and 
> there are sometimes thousands of alternative encodings that could be used 
> to effect the same operation, but only the chosen canonical encoding is 
> disassembled this way.

Aren't you changing topics? Being able to use alternative encodings to
achieve the same effect isn't what we were talking about.

>>>  The alternative mnemonic forms for immediate machine instructions are 
>>> there (NB pinched from the MIPS assembly dialect) to make it easier for 
>>> people to write handcoded assembly, especially where macros are involved, 
>>> either GAS or C preprocessor ones, and not for disassembly.  Conceptually 
>>> they're assembly macros that expand to a single instruction rather than 
>>> aliases.  IOW it's not a bug that those are not considered aliases for 
>>> disassembly.
>>
>> ... may be biased by your MIPS experience.
> 
>  It's rather how the assembly language has been designed (FWIW the RISC-V 
> ISA and assembly dialect have been largely inspired by the MIPS approach).  

Well, such a design imo ought to include a clear statement on uses of
aliases. Iirc at least the 32-bit Arm ARM is very precise about what
aliases exist, and it effectively mandates for at least some of them
that they should be use in disassembly.

As said before, I'd be happy to see things move in about any direction,
just as long as the result is consistent and hence observable behavior
is predictable for users of the assembler and disassembler.

> NB the POWER ISA also uses different mnemonics in its assembly dialect for 
> immediate ALU ("i") or indexed memory ("x") machine operations vs their 
> register or displacement variants.

Of course, as they're permitting plain numbers to represent register
operands.

Jan
  
Maciej W. Rozycki Jan. 25, 2023, 3:22 p.m. UTC | #5
On Wed, 25 Jan 2023, Jan Beulich wrote:

> >  This is however what these instructions have been named in the ISA and 
> > the assembly dialect.  In the case of NOP, MOVE, etc. mnemonics they are 
> > significant assembly idioms (usually mentioned in the ISA manual) and 
> > there are sometimes thousands of alternative encodings that could be used 
> > to effect the same operation, but only the chosen canonical encoding is 
> > disassembled this way.
> 
> Aren't you changing topics? Being able to use alternative encodings to
> achieve the same effect isn't what we were talking about.

 No, it just gives you background as to why some encodings are given 
canonical aliases (used for disassembly) and why some are not.

> >  It's rather how the assembly language has been designed (FWIW the RISC-V 
> > ISA and assembly dialect have been largely inspired by the MIPS approach).  
> 
> Well, such a design imo ought to include a clear statement on uses of
> aliases. Iirc at least the 32-bit Arm ARM is very precise about what
> aliases exist, and it effectively mandates for at least some of them
> that they should be use in disassembly.
> 
> As said before, I'd be happy to see things move in about any direction,
> just as long as the result is consistent and hence observable behavior
> is predictable for users of the assembler and disassembler.

 It's been consistent so far AFAICT for the RISC-V assembly dialect (and 
for that matter for the MIPS one as well).  If you disagree, then you're 
welcome to present your view, but I think the context of libopcodes and 
the binutils mailing list is not the correct place to discuss the assembly 
language syntax.  You'd need to take it to the RISC-V ISA maintainters and 
then we can implement whatever they've agreed to.

  Maciej
  
Andrew Waterman Jan. 26, 2023, 1:29 a.m. UTC | #6
On Wed, Jan 25, 2023 at 7:22 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Wed, 25 Jan 2023, Jan Beulich wrote:
>
> > >  This is however what these instructions have been named in the ISA and
> > > the assembly dialect.  In the case of NOP, MOVE, etc. mnemonics they are
> > > significant assembly idioms (usually mentioned in the ISA manual) and
> > > there are sometimes thousands of alternative encodings that could be used
> > > to effect the same operation, but only the chosen canonical encoding is
> > > disassembled this way.
> >
> > Aren't you changing topics? Being able to use alternative encodings to
> > achieve the same effect isn't what we were talking about.
>
>  No, it just gives you background as to why some encodings are given
> canonical aliases (used for disassembly) and why some are not.
>
> > >  It's rather how the assembly language has been designed (FWIW the RISC-V
> > > ISA and assembly dialect have been largely inspired by the MIPS approach).
> >
> > Well, such a design imo ought to include a clear statement on uses of
> > aliases. Iirc at least the 32-bit Arm ARM is very precise about what
> > aliases exist, and it effectively mandates for at least some of them
> > that they should be use in disassembly.
> >
> > As said before, I'd be happy to see things move in about any direction,
> > just as long as the result is consistent and hence observable behavior
> > is predictable for users of the assembler and disassembler.
>
>  It's been consistent so far AFAICT for the RISC-V assembly dialect (and
> for that matter for the MIPS one as well).  If you disagree, then you're
> welcome to present your view, but I think the context of libopcodes and
> the binutils mailing list is not the correct place to discuss the assembly
> language syntax.  You'd need to take it to the RISC-V ISA maintainters and
> then we can implement whatever they've agreed to.

Precisely specifying the assembly syntax has been a weak spot of the
RISC-V specs, but the general philosophy has been to encourage the use
of aliases in situations that are seemingly obvious (e.g. ret vs. jalr
x0, x1, 0) and to be consistent within a family of instructions (e.g.
since add is an alias for addi, xor had better be an alias for xori).
On the disassembly side, the philosophy has been that more
human-readable aliases should be used when appropriate (e.g. ret, not
jalr x0, x1, 0), unless -Mno-aliases is specified (in which case e.g.
jalr x0, x1, 0 should be printed).

The right place to document this stuff is here:
https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md
(I don't own this spec, so I'd recommend reporting problems or feature
requests on the github issue tracker, rather than replying here).

>
>   Maciej
  
Jan Beulich Jan. 26, 2023, 9:35 a.m. UTC | #7
On 26.01.2023 02:29, Andrew Waterman wrote:
> On Wed, Jan 25, 2023 at 7:22 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>>
>> On Wed, 25 Jan 2023, Jan Beulich wrote:
>>
>>>>  This is however what these instructions have been named in the ISA and
>>>> the assembly dialect.  In the case of NOP, MOVE, etc. mnemonics they are
>>>> significant assembly idioms (usually mentioned in the ISA manual) and
>>>> there are sometimes thousands of alternative encodings that could be used
>>>> to effect the same operation, but only the chosen canonical encoding is
>>>> disassembled this way.
>>>
>>> Aren't you changing topics? Being able to use alternative encodings to
>>> achieve the same effect isn't what we were talking about.
>>
>>  No, it just gives you background as to why some encodings are given
>> canonical aliases (used for disassembly) and why some are not.
>>
>>>>  It's rather how the assembly language has been designed (FWIW the RISC-V
>>>> ISA and assembly dialect have been largely inspired by the MIPS approach).
>>>
>>> Well, such a design imo ought to include a clear statement on uses of
>>> aliases. Iirc at least the 32-bit Arm ARM is very precise about what
>>> aliases exist, and it effectively mandates for at least some of them
>>> that they should be use in disassembly.
>>>
>>> As said before, I'd be happy to see things move in about any direction,
>>> just as long as the result is consistent and hence observable behavior
>>> is predictable for users of the assembler and disassembler.
>>
>>  It's been consistent so far AFAICT for the RISC-V assembly dialect (and
>> for that matter for the MIPS one as well).  If you disagree, then you're
>> welcome to present your view, but I think the context of libopcodes and
>> the binutils mailing list is not the correct place to discuss the assembly
>> language syntax.  You'd need to take it to the RISC-V ISA maintainters and
>> then we can implement whatever they've agreed to.
> 
> Precisely specifying the assembly syntax has been a weak spot of the
> RISC-V specs, but the general philosophy has been to encourage the use
> of aliases in situations that are seemingly obvious (e.g. ret vs. jalr
> x0, x1, 0) and to be consistent within a family of instructions (e.g.
> since add is an alias for addi, xor had better be an alias for xori).

Right, and then along exactly these lines slt{,u} better were aliases of
slti{,u}, with matching preference in disassembly. Hence the patch.

> On the disassembly side, the philosophy has been that more
> human-readable aliases should be used when appropriate (e.g. ret, not
> jalr x0, x1, 0), unless -Mno-aliases is specified (in which case e.g.
> jalr x0, x1, 0 should be printed).
> 
> The right place to document this stuff is here:
> https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md
> (I don't own this spec, so I'd recommend reporting problems or feature
> requests on the github issue tracker, rather than replying here).

Hmm, I'm pretty uncomfortable with discussion of things like that in
forms other than email. You may have noticed this already when I was
pointing out other observations and/or inconsistencies in some of the
spec.

Jan
  
Jan Beulich Jan. 26, 2023, 9:41 a.m. UTC | #8
On 25.01.2023 16:22, Maciej W. Rozycki wrote:
> On Wed, 25 Jan 2023, Jan Beulich wrote:
> 
>>>  This is however what these instructions have been named in the ISA and 
>>> the assembly dialect.  In the case of NOP, MOVE, etc. mnemonics they are 
>>> significant assembly idioms (usually mentioned in the ISA manual) and 
>>> there are sometimes thousands of alternative encodings that could be used 
>>> to effect the same operation, but only the chosen canonical encoding is 
>>> disassembled this way.
>>
>> Aren't you changing topics? Being able to use alternative encodings to
>> achieve the same effect isn't what we were talking about.
> 
>  No, it just gives you background as to why some encodings are given 
> canonical aliases (used for disassembly) and why some are not.
> 
>>>  It's rather how the assembly language has been designed (FWIW the RISC-V 
>>> ISA and assembly dialect have been largely inspired by the MIPS approach).  
>>
>> Well, such a design imo ought to include a clear statement on uses of
>> aliases. Iirc at least the 32-bit Arm ARM is very precise about what
>> aliases exist, and it effectively mandates for at least some of them
>> that they should be use in disassembly.
>>
>> As said before, I'd be happy to see things move in about any direction,
>> just as long as the result is consistent and hence observable behavior
>> is predictable for users of the assembler and disassembler.
> 
>  It's been consistent so far AFAICT for the RISC-V assembly dialect (and 
> for that matter for the MIPS one as well).  If you disagree, then you're 
> welcome to present your view, but I think the context of libopcodes and 
> the binutils mailing list is not the correct place to discuss the assembly 
> language syntax.  You'd need to take it to the RISC-V ISA maintainters and 
> then we can implement whatever they've agreed to.

This, I'm afraid, is again a biased view. ISA and assembly syntax aren't
necessarily connected. See the (odd in my personal view) AT&T syntax which
has been the favorite one in the Unix world for x86, yet is entirely
disconnected from the ISA specifications coming from the relevant x86
vendors (all using so called Intel syntax, which really dates back to
MASM / TASM). Of course an architecture is free to define a "preferred"
assembly language, but I guess that's still distinct from the ISA spec,
and I'm unaware of a specific assembly syntax spec for RISC-V (which
Andrew's reply looks to support).

Jan
  
Andrew Waterman Jan. 26, 2023, 10:20 p.m. UTC | #9
On Thu, Jan 26, 2023 at 1:35 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 26.01.2023 02:29, Andrew Waterman wrote:
> > On Wed, Jan 25, 2023 at 7:22 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
> >>
> >> On Wed, 25 Jan 2023, Jan Beulich wrote:
> >>
> >>>>  This is however what these instructions have been named in the ISA and
> >>>> the assembly dialect.  In the case of NOP, MOVE, etc. mnemonics they are
> >>>> significant assembly idioms (usually mentioned in the ISA manual) and
> >>>> there are sometimes thousands of alternative encodings that could be used
> >>>> to effect the same operation, but only the chosen canonical encoding is
> >>>> disassembled this way.
> >>>
> >>> Aren't you changing topics? Being able to use alternative encodings to
> >>> achieve the same effect isn't what we were talking about.
> >>
> >>  No, it just gives you background as to why some encodings are given
> >> canonical aliases (used for disassembly) and why some are not.
> >>
> >>>>  It's rather how the assembly language has been designed (FWIW the RISC-V
> >>>> ISA and assembly dialect have been largely inspired by the MIPS approach).
> >>>
> >>> Well, such a design imo ought to include a clear statement on uses of
> >>> aliases. Iirc at least the 32-bit Arm ARM is very precise about what
> >>> aliases exist, and it effectively mandates for at least some of them
> >>> that they should be use in disassembly.
> >>>
> >>> As said before, I'd be happy to see things move in about any direction,
> >>> just as long as the result is consistent and hence observable behavior
> >>> is predictable for users of the assembler and disassembler.
> >>
> >>  It's been consistent so far AFAICT for the RISC-V assembly dialect (and
> >> for that matter for the MIPS one as well).  If you disagree, then you're
> >> welcome to present your view, but I think the context of libopcodes and
> >> the binutils mailing list is not the correct place to discuss the assembly
> >> language syntax.  You'd need to take it to the RISC-V ISA maintainters and
> >> then we can implement whatever they've agreed to.
> >
> > Precisely specifying the assembly syntax has been a weak spot of the
> > RISC-V specs, but the general philosophy has been to encourage the use
> > of aliases in situations that are seemingly obvious (e.g. ret vs. jalr
> > x0, x1, 0) and to be consistent within a family of instructions (e.g.
> > since add is an alias for addi, xor had better be an alias for xori).
>
> Right, and then along exactly these lines slt{,u} better were aliases of
> slti{,u}, with matching preference in disassembly. Hence the patch.
>
> > On the disassembly side, the philosophy has been that more
> > human-readable aliases should be used when appropriate (e.g. ret, not
> > jalr x0, x1, 0), unless -Mno-aliases is specified (in which case e.g.
> > jalr x0, x1, 0 should be printed).
> >
> > The right place to document this stuff is here:
> > https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md
> > (I don't own this spec, so I'd recommend reporting problems or feature
> > requests on the github issue tracker, rather than replying here).
>
> Hmm, I'm pretty uncomfortable with discussion of things like that in
> forms other than email. You may have noticed this already when I was
> pointing out other observations and/or inconsistencies in some of the
> spec.

Ah, that's fine, of course.  I (and others) don't mind filing tickets
as a consequence of these email discussions, as long as we know we
need to do so.

>
> Jan
  
Maciej W. Rozycki Jan. 26, 2023, 11:27 p.m. UTC | #10
On Thu, 26 Jan 2023, Jan Beulich wrote:

> This, I'm afraid, is again a biased view. ISA and assembly syntax aren't
> necessarily connected. See the (odd in my personal view) AT&T syntax which
> has been the favorite one in the Unix world for x86, yet is entirely
> disconnected from the ISA specifications coming from the relevant x86
> vendors (all using so called Intel syntax, which really dates back to
> MASM / TASM).

 I don't think this example is really good here.  AFAIK the AT&T syntax 
predates what Microsoft invented with MASM and said company is quite known 
for not respecting established standards, whether formal or de facto.

 NB I find the MASM dialect odd and excessively elaborate with its "WORD 
PTR", etc. constructs even though I knew it first (back in ~1991).  Most 
assembly language dialects I came across are actually closer to x86 AT&T 
than to MASM.

> [...] Of course an architecture is free to define a "preferred"
> assembly language, but I guess that's still distinct from the ISA spec,
> and I'm unaware of a specific assembly syntax spec for RISC-V (which
> Andrew's reply looks to support).

 However I think syntax shouldn't be changed in an ad-hoc manner, because 
we want consistency and existing software to continue working.  We've had 
RISC-V support 6 years now in upstream binutils, so one can't really say 
it's still in its infancy anymore.

 FWIW,

  Maciej
  

Patch

--- a/gas/testsuite/gas/riscv/insn.d
+++ b/gas/testsuite/gas/riscv/insn.d
@@ -45,6 +45,9 @@  Disassembly of section .text:
 [^:]+: R_RISCV_JAL[	]+target
 [^:]+:[ 	]+fa5ff56f[ 	]+jal[ 	]+a0,0 \<target\>
 [^:]+: R_RISCV_JAL[	]+target
+[^:]+:[ 	]+0015a513[ 	]+slt[ 	]+a0,a1,1
+[^:]+:[ 	]+0015b513[ 	]+seqz[ 	]+a0,a1
+[^:]+:[ 	]+0025b513[ 	]+sltu[ 	]+a0,a1,2
 [^:]+:[ 	]+852e[ 	]+mv[ 	]+a0,a1
 [^:]+:[ 	]+0511[ 	]+add[ 	]+a0,a0,4 # .*
 [^:]+:[ 	]+002c[ 	]+add[ 	]+a1,sp,8
@@ -52,9 +55,9 @@  Disassembly of section .text:
 [^:]+:[ 	]+41a8[ 	]+lw[ 	]+a0,64\(a1\)
 [^:]+:[ 	]+c1a8[ 	]+sw[ 	]+a0,64\(a1\)
 [^:]+:[ 	]+8d6d[ 	]+and[ 	]+a0,a0,a1
-[^:]+:[ 	]+d9c9[ 	]+beqz[ 	]+a1,0 \<target\>
+[^:]+:[ 	]+d1d9[ 	]+beqz[ 	]+a1,0 \<target\>
 [^:]+: R_RISCV_RVC_BRANCH[	]+target
-[^:]+:[ 	]+bf41[ 	]+j[ 	]+0 \<target\>
+[^:]+:[ 	]+b751[ 	]+j[ 	]+0 \<target\>
 [^:]+: R_RISCV_RVC_JUMP[	]+target
 [^:]+:[ 	]+68c58543[ 	]+fmadd.s[ 	]+fa0,fa1,fa2,fa3,rne
 [^:]+:[ 	]+68c58543[ 	]+fmadd.s[ 	]+fa0,fa1,fa2,fa3,rne
--- a/gas/testsuite/gas/riscv/insn.s
+++ b/gas/testsuite/gas/riscv/insn.s
@@ -30,6 +30,10 @@  target:
 	.insn uj JAL, a0, target
 	.insn j  JAL, a0, target
 
+	.insn i  OP_IMM, 2, a0, a1, 1
+	.insn i  OP_IMM, 3, a0, a1, 1
+	.insn i  OP_IMM, 3, a0, a1, 2
+
 	.insn cr  C2, 0x8, a0, a1
 	.insn ci  C1, 0x0, a0, 4
 	.insn ciw C0, 0x0, a1, 1
--- a/gas/testsuite/gas/riscv/insn-dwarf.d
+++ b/gas/testsuite/gas/riscv/insn-dwarf.d
@@ -38,49 +38,52 @@  insn.s +29 +0x54.*
 insn.s +30 +0x58.*
 insn.s +31 +0x5c.*
 insn.s +33 +0x60.*
-insn.s +34 +0x62.*
-insn.s +35 +0x64.*
-insn.s +36 +0x66.*
-insn.s +37 +0x68.*
-insn.s +38 +0x6a.*
-insn.s +39 +0x6c.*
-insn.s +40 +0x6e.*
-insn.s +41 +0x70.*
-insn.s +43 +0x72.*
-insn.s +44 +0x76.*
-insn.s +45 +0x7a.*
-insn.s +46 +0x7e.*
-insn.s +47 +0x82.*
-insn.s +48 +0x86.*
-insn.s +49 +0x8a.*
-insn.s +50 +0x8e.*
-insn.s +51 +0x92.*
-insn.s +52 +0x96.*
-insn.s +53 +0x9a.*
-insn.s +54 +0x9e.*
-insn.s +55 +0xa2.*
+insn.s +34 +0x64.*
+insn.s +35 +0x68.*
+insn.s +37 +0x6c.*
+insn.s +38 +0x6e.*
+insn.s +39 +0x70.*
+insn.s +40 +0x72.*
+insn.s +41 +0x74.*
+insn.s +42 +0x76.*
+insn.s +43 +0x78.*
+insn.s +44 +0x7a.*
+insn.s +45 +0x7c.*
+insn.s +47 +0x7e.*
+insn.s +48 +0x82.*
+insn.s +49 +0x86.*
+insn.s +50 +0x8a.*
+insn.s +51 +0x8e.*
+insn.s +52 +0x92.*
+insn.s +53 +0x96.*
+insn.s +54 +0x9a.*
+insn.s +55 +0x9e.*
+insn.s +56 +0xa2.*
 insn.s +57 +0xa6.*
-insn.s +59 +0xaa.*
-insn.s +60 +0xac.*
-insn.s +61 +0xb0.*
-insn.s +62 +0xb6.*
-insn.s +63 +0xbe.*
-insn.s +64 +0xc8.*
-insn.s +65 +0xd4.*
-insn.s +66 +0xea.*
-insn.s +67 +0xec.*
-insn.s +68 +0xf0.*
-insn.s +69 +0xf6.*
-insn.s +70 +0xfe.*
-insn.s +71 +0x108.*
-insn.s +72 +0x114.*
-insn.s +74 +0x12a.*
-insn.s +75 +0x134.*
-insn.s +76 +0x13e.*
-insn.s +77 +0x154.*
-insn.s +78 +0x16a.*
-insn.s +79 +0x180.*
-insn.s +80 +0x196.*
-insn.s +81 +0x1ac.*
-insn.s +- +0x1c2
+insn.s +58 +0xaa.*
+insn.s +59 +0xae.*
+insn.s +61 +0xb2.*
+insn.s +63 +0xb6.*
+insn.s +64 +0xb8.*
+insn.s +65 +0xbc.*
+insn.s +66 +0xc2.*
+insn.s +67 +0xca.*
+insn.s +68 +0xd4.*
+insn.s +69 +0xe0.*
+insn.s +70 +0xf6.*
+insn.s +71 +0xf8.*
+insn.s +72 +0xfc.*
+insn.s +73 +0x102.*
+insn.s +74 +0x10a.*
+insn.s +75 +0x114.*
+insn.s +76 +0x120.*
+insn.s +78 +0x136.*
+insn.s +79 +0x140.*
+insn.s +80 +0x14a.*
+insn.s +81 +0x160.*
+insn.s +82 +0x176.*
+insn.s +83 +0x18c.*
+insn.s +84 +0x1a2.*
+insn.s +85 +0x1b8.*
+insn.s +- +0x1ce
 #pass
--- a/gas/testsuite/gas/riscv/insn-na.d
+++ b/gas/testsuite/gas/riscv/insn-na.d
@@ -36,6 +36,9 @@  Disassembly of section .text:
 [^:]+:[ 	]+00fff537[ 	]+lui[ 	]+a0,0xfff
 [^:]+:[ 	]+fa9ff56f[ 	]+jal[ 	]+a0,0 \<target\>
 [^:]+:[ 	]+fa5ff56f[ 	]+jal[ 	]+a0,0 \<target\>
+[^:]+:[ 	]+0015a513[ 	]+slti[ 	]+a0,a1,1
+[^:]+:[ 	]+0015b513[ 	]+sltiu[ 	]+a0,a1,1
+[^:]+:[ 	]+0025b513[ 	]+sltiu[ 	]+a0,a1,2
 [^:]+:[ 	]+852e[ 	]+c\.mv[ 	]+a0,a1
 [^:]+:[ 	]+0511[ 	]+c\.addi[ 	]+a0,4 # .*
 [^:]+:[ 	]+002c[ 	]+c\.addi4spn[ 	]+a1,sp,8
@@ -43,8 +46,8 @@  Disassembly of section .text:
 [^:]+:[ 	]+41a8[ 	]+c\.lw[ 	]+a0,64\(a1\)
 [^:]+:[ 	]+c1a8[ 	]+c\.sw[ 	]+a0,64\(a1\)
 [^:]+:[ 	]+8d6d[ 	]+c\.and[ 	]+a0,a1
-[^:]+:[ 	]+d9c9[ 	]+c\.beqz[ 	]+a1,0 \<target\>
-[^:]+:[ 	]+bf41[ 	]+c\.j[ 	]+0 \<target\>
+[^:]+:[ 	]+d1d9[ 	]+c\.beqz[ 	]+a1,0 \<target\>
+[^:]+:[ 	]+b751[ 	]+c\.j[ 	]+0 \<target\>
 [^:]+:[ 	]+68c58543[ 	]+fmadd\.s[ 	]+fa0,fa1,fa2,fa3,rne
 [^:]+:[ 	]+68c58543[ 	]+fmadd\.s[ 	]+fa0,fa1,fa2,fa3,rne
 [^:]+:[ 	]+68c58543[ 	]+fmadd\.s[ 	]+fa0,fa1,fa2,fa3,rne
--- a/opcodes/riscv-opc.c
+++ b/opcodes/riscv-opc.c
@@ -448,12 +448,12 @@  const struct riscv_opcode riscv_opcodes[
 {"snez",        0, INSN_CLASS_I, "d,t",       MATCH_SLTU, MASK_SLTU|MASK_RS1, match_opcode, INSN_ALIAS },
 {"sltz",        0, INSN_CLASS_I, "d,s",       MATCH_SLT, MASK_SLT|MASK_RS2, match_opcode, INSN_ALIAS },
 {"sgtz",        0, INSN_CLASS_I, "d,t",       MATCH_SLT, MASK_SLT|MASK_RS1, match_opcode, INSN_ALIAS },
-{"slti",        0, INSN_CLASS_I, "d,s,j",     MATCH_SLTI, MASK_SLTI, match_opcode, 0 },
 {"slt",         0, INSN_CLASS_I, "d,s,t",     MATCH_SLT, MASK_SLT, match_opcode, 0 },
 {"slt",         0, INSN_CLASS_I, "d,s,j",     MATCH_SLTI, MASK_SLTI, match_opcode, INSN_ALIAS },
-{"sltiu",       0, INSN_CLASS_I, "d,s,j",     MATCH_SLTIU, MASK_SLTIU, match_opcode, 0 },
+{"slti",        0, INSN_CLASS_I, "d,s,j",     MATCH_SLTI, MASK_SLTI, match_opcode, 0 },
 {"sltu",        0, INSN_CLASS_I, "d,s,t",     MATCH_SLTU, MASK_SLTU, match_opcode, 0 },
 {"sltu",        0, INSN_CLASS_I, "d,s,j",     MATCH_SLTIU, MASK_SLTIU, match_opcode, INSN_ALIAS },
+{"sltiu",       0, INSN_CLASS_I, "d,s,j",     MATCH_SLTIU, MASK_SLTIU, match_opcode, 0 },
 {"sgt",         0, INSN_CLASS_I, "d,t,s",     MATCH_SLT, MASK_SLT, match_opcode, INSN_ALIAS },
 {"sgtu",        0, INSN_CLASS_I, "d,t,s",     MATCH_SLTU, MASK_SLTU, match_opcode, INSN_ALIAS },
 {"sb",          0, INSN_CLASS_I, "t,q(s)",    MATCH_SB, MASK_SB, match_opcode, INSN_DREF|INSN_1_BYTE },