Skip to content
This repository has been archived by the owner on Aug 17, 2022. It is now read-only.

Add remaining b-ext insns #255

Conversation

pz9115
Copy link
Contributor

@pz9115 pz9115 commented Apr 8, 2021

Add remain b-ext insns with zbe zbf zbm zbr zbs zbt and their combine

@pz9115 pz9115 force-pushed the riscv-binutils-experiment branch 2 times, most recently from e761ec1 to a224df9 Compare April 8, 2021 04:20
@pz9115 pz9115 force-pushed the riscv-binutils-experiment branch from a224df9 to 73f6b56 Compare April 8, 2021 04:23
gas/config/tc-riscv.c Outdated Show resolved Hide resolved
include/opcode/riscv.h Outdated Show resolved Hide resolved
include/opcode/riscv.h Outdated Show resolved Hide resolved
@pz9115 pz9115 force-pushed the riscv-binutils-experiment branch from f3fde2b to 41b5aa9 Compare April 8, 2021 05:05
@pz9115 pz9115 changed the title Add remain b-ext insns Add remaining b-ext insns Apr 8, 2021
gas/config/tc-riscv.c Outdated Show resolved Hide resolved
@pz9115 pz9115 force-pushed the riscv-binutils-experiment branch from 1f98bde to 058cf13 Compare April 8, 2021 09:19
Copy link
Collaborator

@Nelson1225 Nelson1225 left a comment

Choose a reason for hiding this comment

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

Seems like we also need the b pseudos, but they can be added in the future patches. Otherwise, this looks OK to me.

bfd/elfxx-riscv.c Outdated Show resolved Hide resolved
@@ -1,4 +1,4 @@
#as: -march=rv64i_zba_zbb_zbc
#as: -march=rv64i_zba_zbb_zbc_zbe_zbf_zbm_zbp_zbr_zbs_zbt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we can use rv64ib, but this doesn't really matter

@kito-cheng
Copy link
Collaborator

@pz9115 Oh I guess I forgot to confirm with you in the afternoon meeting, does binutils/gas testsuite is all pass now?

@pz9115
Copy link
Contributor Author

pz9115 commented Apr 22, 2021

I am still debugging, sorry for that.

@pz9115
Copy link
Contributor Author

pz9115 commented Apr 22, 2021

I used the --debug option, but it seems not work rightly, here is the log:

=====================================================
GNU Make 4.2.1
Built for x86_64-pc-linux-gnu
Copyright (C) 1988-2016 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Reading makefiles...
Updating makefiles....
Updating goal targets....
File 'report-binutils' does not exist.
File 'report-binutils-newlib' does not exist.
Must remake target 'report-binutils-newlib'.
/root/b-ext/scripts/testsuite-filter binutils newlib
/root/b-ext/test/allowlist
find build-binutils-newlib/ -name *.sum |paste -sd "," -
=== gas: Unexpected fails for rv64gc_zba_zbb_zbc_zbe_zbf_zbm_zbp_zbr_zbs_zbt lp64d medlow ===
FAIL: gas/riscv/b-ext-64
FAIL: gas/riscv/b-ext

           ========= Summary of binutils testsuite =========
                        | # of unexpected case
                        |     binutils |           ld |          gas |

rv64gc_zba_zbb_zbc_zbe_zbf_zbm_zbp_zbr_zbs_zbt/ lp64d/ medlow | 0 | 0 | 2 |
make: *** [Makefile:896: report-binutils-newlib] Error 1

=====================================================

@kito-cheng
Copy link
Collaborator

I guess it need to pass --debug flag to dejagnu, @Nelson1225 could you describe how to do that?

@jim-wilson
Copy link
Collaborator

I think I am missing some context here on the dejagnu failure. I didn't see the original question.

Normally all you need to do is look at the gas/testsuite/gas.log file in the build tree to see what was run and what failed. Just search for the FAIL string and ignore XFAIL. If using riscv-gnu-toolchain, this is inside build-binutils-{newlib,linux}.

If you need more info in the gas.log file, you can add a -v (verbose) option to RUNTESTFLAGS. The more -v options you add the more verbose the output. I think it goes up to 4 -v options. -v is sometimes useful for gcc, if running the testsuite on a board and you need to see communication between dejagnu and the target board. But for running the gas testsuite I doubt that it is necessary. If using riscv-gnu-toolchain, it sets RUNTESTFLAGS itself, to set the target board to a simulator so that testcases can be executed. So you will need to hack the Makefile to add the -v options. Or else rewrite the source Makefile.in so that the user has a way to add options to dejagnu RUNTESTFLAGS.

@pz9115
Copy link
Contributor Author

pz9115 commented Apr 22, 2021

Thank you Jim, I will try it.

@Nelson1225
Copy link
Collaborator

Nelson1225 commented Apr 23, 2021

https://www.gnu.org/software/dejagnu/manual/Debugging-a-test-case.html

For example,
RUNTESTFLAGS="-v -v -v --debug" make check-gas/ld/binutils -j8

@pz9115
Copy link
Contributor Author

pz9115 commented Apr 23, 2021

I had fix the testcase errors, now it works right, thank you all.

Add Pseudo-instructions of 'grevi' ‘gorci’ 'shfli/unshfli' in spec table 2.4-2.9.
@pz9115
Copy link
Contributor Author

pz9115 commented Jun 3, 2021

Sorry for I missed of adding those instructions in table, I had added them.

@jim-wilson
Copy link
Collaborator

Thanks. It looks like you marked them all as zbb. Only rev8 and orc.b are zbb. The rest are zbp, same as grev/grevi and gorc/gorc.i.

Update testcase with pseudo-instructions and fix their class set in ZBP, except rev8 and orc.b follow to ZBB.
@pz9115
Copy link
Contributor Author

pz9115 commented Jun 3, 2021

Thank you Jim, I had update the testcase and fix the class set.

@pz9115
Copy link
Contributor Author

pz9115 commented Jun 4, 2021

I tested after the pseudo-instructions added and found three bugs here,
The instruction 'grevi', 'gorci', 'shfli' was error replaced by 'rev2.n', 'orc2.n', 'zip.n' after disassmble
I'm not sure how this happened(with the instruction 'unshfli' is correct), here is the test file and test result in rv64.
The instruction declare in riscv-opc.c L98 L939

test source (the error part form b-ext-64.s)

grevi    a0, a1, 2
gorci    a0, a1, 2
shfli    a0, a1, 2

test step:

riscv64-unknown-elf-gcc -c pse_ero.s
riscv64-unknown-elf-objdump -d pse_ero.o

test result:

Disassembly of section .text:

0000000000000000 <.text>:
   0:   6825d513                rev2.n  a0,a1
   4:   2825d513                orc2.n  a0,a1
   8:   08259513                zip.n   a0,a1

@kito-cheng
Copy link
Collaborator

It seems not bugs? I guess you just need -Mno-aliases?

@pz9115 pz9115 force-pushed the riscv-binutils-experiment branch from a13c4d1 to a48a9e8 Compare June 4, 2021 09:13
@pz9115
Copy link
Contributor Author

pz9115 commented Jun 4, 2021

I use the arch and update the testcases, thanks.

@pz9115 pz9115 force-pushed the riscv-binutils-experiment branch 2 times, most recently from ebfbb3d to ade9f69 Compare June 7, 2021 03:31
Update testcases with aliases for pseudo-instructions.
@pz9115 pz9115 force-pushed the riscv-binutils-experiment branch from ade9f69 to 77ffe98 Compare June 7, 2021 03:57
Copy link
Collaborator

@Nelson1225 Nelson1225 left a comment

Choose a reason for hiding this comment

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

Same GNU coding standard problems. It would be great if these draft stuff are contributed to FSF integration branch, so that the coding standards are important. But if you just want to keep them downstream, then I have no objection to merge them into riscv-binutils-experiment branch.

bfd/elfxx-riscv.c Outdated Show resolved Hide resolved
bfd/elfxx-riscv.c Outdated Show resolved Hide resolved
bfd/elfxx-riscv.c Outdated Show resolved Hide resolved
gas/config/tc-riscv.c Outdated Show resolved Hide resolved
#define MATCH_SHAMT_REV8_32 (0b11000 << OP_SH_SHAMT)
#define MATCH_SHAMT_REV8_64 (0b111000 << OP_SH_SHAMT)
#define MATCH_SHAMT_ORC_B (0b00111 << OP_SH_SHAMT)
#define MATCH_SHAMT_REV_32 (0b11111 << OP_SH_SHAMT)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suggest encode the (0b11111 << OP_SH_SHAMT) into riscv_opcodes directly, since these defines looks redundant.

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 am not sure how to do this, it is mean compute the result "MATCH_GORCI | MATCH_SHAMT_REV_32" and send it into riscv_opcodes or compute the result (0b11111 << OP_SH_SHAMT) and send it "MATCH_GORCI | res" ?

opcodes/riscv-opc.c Outdated Show resolved Hide resolved
pz9115 and others added 7 commits June 9, 2021 19:16
Fix the indents problems and update INSN_ALIAS FLAG for pseudo instructions.
Use 5bit coding for imm shamt part
#define MATCH_FSRW 0x400503B
#define MATCH_FSRIW 0x400501B
This bug is locate to this commit:
85dd8e0

Where operands case 'r' was accidentally put under
case 'C': /* RVC */
	  switch (*++d)
	    {...}
which caused  print_insn_args() to
print (info->stream, _("# internal error, undefined modifier (V%c)"),
		     *d);
For shfli/unshfli alias, 5bit << OP_SH_SHAMT is enough
riscv_fpr_names to riscv_gpr_names
@pz9115 pz9115 force-pushed the riscv-binutils-experiment branch from e1d39fc to 62e1569 Compare July 12, 2021 06:16
Update testcase and fix merge errors with duplicate defines.
@pz9115 pz9115 force-pushed the riscv-binutils-experiment branch 2 times, most recently from ae3bfc5 to 2420bb0 Compare July 12, 2021 07:47
@pz9115 pz9115 deleted the branch riscvarchive:riscv-binutils-experiment August 19, 2021 09:09
@pz9115 pz9115 closed this Aug 19, 2021
@pz9115 pz9115 deleted the riscv-binutils-experiment branch August 19, 2021 09:09
@pz9115
Copy link
Contributor Author

pz9115 commented Aug 23, 2021

We had merge b-ext 1.0.0-rc with k-ext 1.0.0-rc in #254, you can find the dev branch for old version use in https://github.com/pz9115/riscv-binutils-gdb/tree/riscv-binutils-b-ext, thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants