RISC-V: Using fmv.x.w/fmv.w.x rather than fmv.x.s/fmv.s.x

Message ID 20200218055734.60096-1-kito.cheng@sifive.com
State New
Headers show
Series
  • RISC-V: Using fmv.x.w/fmv.w.x rather than fmv.x.s/fmv.s.x
Related show

Commit Message

Kito Cheng Feb. 18, 2020, 5:57 a.m.
- fmv.x.s/fmv.s.x renamed to fmv.x.w/fmv.w.x in the latest RISC-V ISA
   manual.

 - Tested rv32gc/rv64gc on bare-metal with qemu.

ChangeLog

gcc/

Kito Cheng  <kito.cheng@sifive.com>

	* config/riscv/riscv.c (riscv_output_move) Using fmv.x.w/fmv.w.x
	rather than fmv.x.s/fmv.s.x.
---
 gcc/config/riscv/riscv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.25.0

Comments

Jim Wilson Feb. 18, 2020, 11:32 p.m. | #1
On Mon, Feb 17, 2020 at 9:57 PM Kito Cheng <kito.cheng@sifive.com> wrote:
>         * config/riscv/riscv.c (riscv_output_move) Using fmv.x.w/fmv.w.x

>         rather than fmv.x.s/fmv.s.x.


Looks good to me also.

By the way, since you are listed as one of the riscv port maintainers,
you could make changes like this without asking for a review.  Maybe
ask for a review for the more complicated patches, and commit the
simpler ones before sending the patch.  And indicate in the patch
whether you are asking for a review or already committed it.

Jim
Kito Cheng Feb. 19, 2020, 5:09 a.m. | #2
Committed.

Hi Jim:

Thanks for your review and reply, I'll commit directly for such simple
patch in future :)


On Wed, Feb 19, 2020 at 7:33 AM Jim Wilson <jimw@sifive.com> wrote:
>

> On Mon, Feb 17, 2020 at 9:57 PM Kito Cheng <kito.cheng@sifive.com> wrote:

> >         * config/riscv/riscv.c (riscv_output_move) Using fmv.x.w/fmv.w.x

> >         rather than fmv.x.s/fmv.s.x.

>

> Looks good to me also.

>

> By the way, since you are listed as one of the riscv port maintainers,

> you could make changes like this without asking for a review.  Maybe

> ask for a review for the more complicated patches, and commit the

> simpler ones before sending the patch.  And indicate in the patch

> whether you are asking for a review or already committed it.

>

> Jim
Christophe Lyon via Gcc-patches March 17, 2020, 9:42 p.m. | #3
On Tue, 18 Feb 2020, Kito Cheng wrote:

>  - fmv.x.s/fmv.s.x renamed to fmv.x.w/fmv.w.x in the latest RISC-V ISA

>    manual.


 The new mnemonics have been supported by GAS for a little while now and 
the old ones have been retained, however this is still a change that 
breaks backwards compatibility.  So I wonder if we shouldn't have an 
autoconf test included for this feature, and either resort to wiring GCC 
to keep using the old mnemonics or bail out at GCC compilation time if 
GAS is found not to handle the new ones.

 At the very least I think we ought to document the minimum version of 
binutils now required by GCC for RISC-V support.

  Maciej
Jim Wilson March 18, 2020, 11:19 p.m. | #4
On Tue, Mar 17, 2020 at 2:42 PM Maciej W. Rozycki <macro@wdc.com> wrote:
> On Tue, 18 Feb 2020, Kito Cheng wrote:

> >  - fmv.x.s/fmv.s.x renamed to fmv.x.w/fmv.w.x in the latest RISC-V ISA

> >    manual.

>

>  The new mnemonics have been supported by GAS for a little while now and

> the old ones have been retained, however this is still a change that

> breaks backwards compatibility.  So I wonder if we shouldn't have an

> autoconf test included for this feature, and either resort to wiring GCC

> to keep using the old mnemonics or bail out at GCC compilation time if

> GAS is found not to handle the new ones.

>

>  At the very least I think we ought to document the minimum version of

> binutils now required by GCC for RISC-V support.


The new opcodes were added to gas in 2017-09-27, and I can't recommend
using any binutils or gcc release that predates 2018-01-01 because
they are all known to be buggy, or incompatible with the current ISA
definition.  So I don't see any need for a configure test for this
change.  Anyone missing the new instructions in gas has bigger
problems to worry about.

Speaking of which, the ISA is unfortunately still making the
occasional backwards incompatible change, though I and others keep
complaining about that.  There was a break between the privilege spec
1.9 and 1.9.1, and there was a break between the priv spec 1.9.1 and
1.11.  Though I'm told that the goal is no breaks from priv spec 1.10
forward, and that was released 2017-05-17 so we can't properly support
any priv spec predating that.  Fortunately the priv spec only affects
people doing OS level work, or bare metal work.  But there have been
some unpriv isa spec breaks too though not in any critical areas.  For
instance some instructions like fence.i and the csr* insns have been
moved out of the base ISA into extensions and we haven't decided how
to handle that yet.  binutils and gcc still think they are part of the
base ISA.  The syntax for specifying architecture extensions changed,
sx extensions were dropped, and it used to be that x came before s but
now s comes before x.  We decided to drop support for ISA strings
supported before the 2019-12-13 unpriv spec because there were no
known uses of s or sx extensions that would be affected by the change,
and it was too complicated trying to support both the old and new
syntax.  I realize that you would like perfect compatibility, but that
won't be possible until the RISC-V ecosystem is more mature.  At least
for the linux support, we are being very careful not to change
anything that would break linux.  That is just for rv64 linux though.
rv32 linux is not upstream yet, and still adding breaking changes
because of Y2038 work.  There was a very minor ABI change last year
that affects rv64 linux, but it was obscure enough that no one testing
gcc-10 seems to have been affected by it.  There are also no official
distro releases that we need backward compatibility with yet.

As for the minimum binutils version, I would strongly recommend the
most recent one released before the gcc release that you are using,
though it is likely than anything back to 2018-01-01 would work, just
not as well.

Jim
Christophe Lyon via Gcc-patches March 18, 2020, 11:55 p.m. | #5
On Wed, 18 Mar 2020, Jim Wilson wrote:

> >  The new mnemonics have been supported by GAS for a little while now and

> > the old ones have been retained, however this is still a change that

> > breaks backwards compatibility.  So I wonder if we shouldn't have an

> > autoconf test included for this feature, and either resort to wiring GCC

> > to keep using the old mnemonics or bail out at GCC compilation time if

> > GAS is found not to handle the new ones.

> >

> >  At the very least I think we ought to document the minimum version of

> > binutils now required by GCC for RISC-V support.

> 

> The new opcodes were added to gas in 2017-09-27, and I can't recommend

> using any binutils or gcc release that predates 2018-01-01 because

> they are all known to be buggy, or incompatible with the current ISA

> definition.  So I don't see any need for a configure test for this

> change.  Anyone missing the new instructions in gas has bigger

> problems to worry about.


 Fair enough.

> As for the minimum binutils version, I would strongly recommend the

> most recent one released before the gcc release that you are using,

> though it is likely than anything back to 2018-01-01 would work, just

> not as well.


 For me it's not an issue as I actively work on the toolchain and keep all 
checkouts close to the current tips of the respective master branches.  
However binary package maintainers or end users of the toolchain need to 
know the dependencies between component versions whether they want to 
build the pieces from sources or combine them from prebuilt packages.

 Our installation instructions state binutils 2.28 as the requirement for 
all the RISC-V targets, however the change for fmv.x.w/fmv.w.x instruction 
support was only added in the binutils 2.30 development cycle.

  Maciej

Patch

diff --git a/gcc/config/riscv/riscv.c b/gcc/config/riscv/riscv.c
index ee51ad7ce1e..5ef74acb56d 100644
--- a/gcc/config/riscv/riscv.c
+++ b/gcc/config/riscv/riscv.c
@@ -1917,7 +1917,7 @@  riscv_output_move (rtx dest, rtx src)
   if (dest_code == REG && GP_REG_P (REGNO (dest)))
     {
       if (src_code == REG && FP_REG_P (REGNO (src)))
-	return dbl_p ? "fmv.x.d\t%0,%1" : "fmv.x.s\t%0,%1";
+	return dbl_p ? "fmv.x.d\t%0,%1" : "fmv.x.w\t%0,%1";
 
       if (src_code == MEM)
 	switch (GET_MODE_SIZE (mode))
@@ -1954,7 +1954,7 @@  riscv_output_move (rtx dest, rtx src)
 	  if (FP_REG_P (REGNO (dest)))
 	    {
 	      if (!dbl_p)
-		return "fmv.s.x\t%0,%z1";
+		return "fmv.w.x\t%0,%z1";
 	      if (TARGET_64BIT)
 		return "fmv.d.x\t%0,%z1";
 	      /* in RV32, we can emulate fmv.d.x %0, x0 using fcvt.d.w */