Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add c.sext.w and zext.w #290

Merged
merged 1 commit into from
Oct 25, 2024
Merged

Add c.sext.w and zext.w #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions rv64_zba
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ sh1add.uw rd rs1 rs2 31..25=16 14..12=2 6..2=0x0E 1..0=3
sh2add.uw rd rs1 rs2 31..25=16 14..12=4 6..2=0x0E 1..0=3
sh3add.uw rd rs1 rs2 31..25=16 14..12=6 6..2=0x0E 1..0=3
slli.uw rd rs1 31..26=2 shamtd 14..12=1 6..2=0x06 1..0=3

$pseudo_op rv64_zba::add.uw zext.w rd rs1 31..25=4 24..20=0 14..12=0 6..2=0x0E 1..0=3
2 changes: 2 additions & 0 deletions rv64_zcb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
c.zext.w rd_rs1_p 1..0=1 15..13=4 12..10=7 6..5=3 4..2=4

$pseudo_op rv64_c::c.addiw c.sext.w rd_rs1_n0 15..13=1 12=0 6..2=0 1..0=1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c.addiw is part of the C extension, not Zcb, so this should be moved to rv64_c.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be wrong here, but c.addiw is identified as part of the c extension by rv64_c::c.addiw and c.sext.w pseudo instruction appears to only be available in rv64_zcb. This is the case in the binutils and in the ISA manual

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like Binutils picks up sext.w as a pseudoinstruction for c.addiw when the C extension is enabled: https://github.com/bminor/binutils-gdb/blob/2754d75a11556577fcddf23718bfbff7d0489a3f/opcodes/riscv-opc.c#L580

But it only picks up c.sext.w as a pseudoinstruction for c.addiw when Zcb is enabled.

This all seems pretty illogical to me, but I guess we are stuck with the binutils behavior at this point for compatibility reasons, right @kito-cheng?

Copy link
Contributor Author

@AFOliveira AFOliveira Oct 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it seems ilogical, my guess is binutils is doing that because the pseudo-instruction is only mentioned as a note on Zcb, maybe if the ISA clarified that, we could change all the others.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aswaterman Just a ping. Do we want to keep this on hold?
Technically, the PR is according to the rest of the sources. if anything changes in the meantime, I'll be happy to address those changes and PR any update.

Loading