Implement the compressed instructions #5

eriktai · 2020-01-11T08:59:40Z

Some modification of rv32emu to support C ext.

Add a new variable named insn_type to distinguish the 16-bit and 32-bit instruction.
Modify the unsigned char get_insn32(uint32_t, uint32_t*) to return the type of instruction and then assign the pointer the machine code of the instruction.
Modify the instruction offset, instead of always proceeding 4 bytes, it will depend on the type of instruction.
- 16-bits : 2 bytes
- 32-bits : 4 bytes
Rearrange the offset of outputing the signature.
Update the comparing function, which can compare the groundtruth and output of signature of this emulator for debugging.
It is also the version of fixing start address of stack pointer.
Imp and finish the test of all 32-bit version compressed instructions except those 64-bit version, 128-bit version and F ext.

Future work

I would like to imp the floating point extension.

jserv · 2020-01-11T09:14:09Z

Remove the generated files and rework!

jserv · 2020-01-11T09:15:27Z

CSRs.h

+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */


The range from Line 1 to 68 is not necessary. Remove these lines.

Sor, I may forget about it.

jserv · 2020-01-11T09:16:55Z

Hint: use git rebase -i to rework these Git commits.

eriktai · 2020-01-11T09:29:52Z

Sorry, I have a question.
Do I need to rework all the commit messages?

jserv · 2020-01-11T10:46:56Z

Sorry, I have a question.
Do I need to rework all the commit messages?

Yes, all commits should be reworked. You shall squash the similar changes and make the commits clear.

eriktai · 2020-01-11T12:06:42Z

Hello, Jserv
I have reworked these commit messages.
Please check, thank you~

jserv · 2020-01-11T14:37:08Z

After reworking Git commits with git rebase -i, you should perform git push --force for this branch.

jserv · 2020-01-11T14:38:07Z

Makefile

@@ -1,19 +1,60 @@
 BINS = emu-rv32i test1
+TEST_TARGETS = \


Send another pull request for adding RISC-V test cases.

jserv · 2020-01-11T14:39:07Z

Ensure no generated files such as ELF, log, dump files in Git repository.

jserv · 2020-01-12T04:24:11Z

You have to rebase master branch of upstream. And then, force push.

jserv · 2020-01-12T04:24:57Z

Don't use git merge. Use git rebase instead.

jserv · 2020-01-12T04:26:33Z

Reference instructions:

set upstream: git remote add upstream https://github.com/sysprog21/rv32emu && git fetch upstream
Perform rebase: git rebase upstream/master
Fix the conflicts.

jserv · 2020-01-12T04:27:57Z

CSRs.h

+
+#include <stdint.h>
+
+


Can you introduce a dedicated structure for preserving the following fields?

jserv · 2020-01-12T04:28:52Z

emu-rv32i.c

+After this you can run it with the emulator like this:
+emu-rv32i zephyr/zephyr.elf
+original copyright:
+*/


Remove the range from Line 1 to Line 35.

jserv · 2020-01-12T04:29:07Z

emu-rv32i.c

-// #define DEBUG_OUTPUT
-// #define DEBUG_EXTRA
+#define DEBUG_OUTPUT
+#define DEBUG_EXTRA


Don't do that.

jserv · 2020-01-12T04:29:29Z

emu-rv32i.c

+#define DEBUG_EXTRA
+
+#if 1
+#define RV32C


RV32C should be assigned via Makefile

jserv · 2020-01-12T04:31:16Z

emu-rv32i.c

-    ""};
+#define C_STATS_NUM 64
+
+#ifndef RV32C


I would like to use the following:

char statnames[STATS_NUM + C_STATS_NUM][16] = { ... /* RV32I instructions */ #if defined(RV32C) ... /* RV32C specific instructions */ #endif }

OK! It's more structural.

jserv · 2020-01-12T04:32:15Z

emu-rv32i.c

-    maxmemr = maxmemw = 0;
+  for(int i=0;i<STATS_NUM;i++) stats[i]=0;
+#ifdef RV32C
+  for(int i=0;i<C_STATS_NUM;i++) stats[i+64] = 0;


Ditto. You can define macro for the range. Thus, we don't have to duplicate the loops.

jserv · 2020-01-12T04:34:20Z

Before submitting the changes, ensure the completion of executing clang-format -i *.[ch]

jserv · 2020-01-16T17:09:59Z

Reworked yet?

eriktai · 2020-01-19T12:20:41Z

Not yet, I will update it tomorrow. Jim Huang <[email protected]> 於 2020年1月17日週五上午1:10寫道：

…

Reworked yet? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <sysprog21/rv32emu#5?email_source=notifications&email_token=AI4KYTPOEE7CU32GFLJLIVDQ6CIGPA5CNFSM4KFQ6NU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJE2HLQ#issuecomment-575251374>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AI4KYTOCJ6566MQXVGGPNSDQ6CIGPANCNFSM4KFQ6NUQ> .

Add the method of how to test file

Add the C.JAL and C.ADDIW insn

Delete the unnecessary file

eriktai · 2020-01-20T14:37:13Z

Hello Jserv,
Sorry about the absence a few days ago.
I have rebased all commits.

jserv · 2020-01-20T21:02:32Z

.gitignore

+sig.txt
+rom.v
+debug.txt
+CSR.h


CSR.h was tracked in GIT repository.

jserv · 2020-01-20T21:03:23Z

emu-rv32i.c

@@ -23,6 +24,8 @@
 // #define DEBUG_OUTPUT
 // #define DEBUG_EXTRA

+
+/* added by Shaos: */


You don't have to mention the contributor name inside source file.

jserv · 2020-01-20T21:03:45Z

emu-rv32i.c

@@ -140,7 +163,7 @@ uint64_t mtimecmp;
 /* virtual start address for index 0 in the ram array */
 uint32_t ram_start;

-/* program entry point */
+/* Program entry point */


Don't do that. Let's minimize the changes.

jserv · 2020-01-20T21:04:11Z

emu-rv32i.c

 /* CPU state */
 uint32_t pc;
 uint32_t next_pc;
 uint32_t insn;
 uint32_t reg[32];

+


Ditto. Minimize the changes.

jserv · 2020-01-20T21:10:29Z

Thank @kksweet8845 again for contributing to this project. However, we shall always merge commits with better Git commit messages for further development. Please split this pull request into the following:

New pull request for rv32i conformance tests;
Once the above is merged, modify the pull request to concentrate on RV32C;

Changes regarding CSR instructions should be addressed in part 1.

jserv reviewed Jan 11, 2020

View reviewed changes

Makefile

@@ -1,19 +1,60 @@

BINS = emu-rv32i test1

TEST_TARGETS = \

Copy link

Contributor

jserv Jan 11, 2020

Choose a reason for hiding this comment

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

Send another pull request for adding RISC-V test cases.

eriktai force-pushed the master branch from 64abd89 to e429a5c Compare January 11, 2020 14:56

jserv reviewed Jan 12, 2020

View reviewed changes

CSRs.h Outdated

#include <stdint.h>

Copy link

Contributor

jserv Jan 12, 2020

Choose a reason for hiding this comment

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

Can you introduce a dedicated structure for preserving the following fields?

jserv reviewed Jan 12, 2020

View reviewed changes

emu-rv32i.c Outdated

// #define DEBUG_OUTPUT

// #define DEBUG_EXTRA

#define DEBUG_OUTPUT

#define DEBUG_EXTRA

Copy link

Contributor

jserv Jan 12, 2020

Choose a reason for hiding this comment

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

Don't do that.

jserv reviewed Jan 12, 2020

View reviewed changes

emu-rv32i.c Outdated

#define DEBUG_EXTRA

#if 1

#define RV32C

Copy link

Contributor

jserv Jan 12, 2020

Choose a reason for hiding this comment

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

RV32C should be assigned via Makefile

jserv reviewed Jan 12, 2020

View reviewed changes

jserv changed the title ~~Finishing the compressed instruction of rv32emu~~ Implement the compressed instructions Jan 16, 2020

jserv and others added 7 commits January 20, 2020 22:01

Perform consistent indention

25e917c

Create the CSRs.h for CSR

275aa51

Add the compressed C.NOP and C.ADDI

5b853e5

Add the C.JAL and C.ADDIW insn

7a9c406

Modified some implementataion detail to support compressed instruction

dc68602

Add the new function for signature validation

6331c15

Modified the gitignore

5b143f9

eriktai and others added 17 commits January 20, 2020 22:09

Update README.txt

25705a3

Add the method of how to test file

Update README.txt

5036e85

Modified the makefile to enable to check the validation

965e0d8

Update README.md

3ce2880

Finish the Compressed instruction imp

0bff3d3

Add the compressed C.NOP, C.ADDi and modify the get_insn32 function

28fa1d5

Add the C.JAL and C.ADDIW insn

Modified some implementataion detail to support compressed instruction

79c72f1

Finish the rv32imc opcode 1 insn

5483b52

Add the new function for signature validation

4232314

Modified the gitignore

fcd3ebf

Delete the unnecessary file

Finish the Compressed instruction imp

6f8c583

Fix unknow problem of merging from previous commit

743ad85

Remove missing debugging info

3440e94

Modified the README.md about testing the test-cases

83f3413

Make emu-rv32i be compatible to sysprog21

6672f8d

Rework to a better style

75a75dc

Rework to a final version of emu-rv32i.c

ce9f330

eriktai force-pushed the master branch from 086c008 to ce9f330 Compare January 20, 2020 14:31

jserv reviewed Jan 20, 2020

View reviewed changes

.gitignore

sig.txt

rom.v

debug.txt

CSR.h

Copy link

Contributor

jserv Jan 20, 2020

Choose a reason for hiding this comment

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

CSR.h was tracked in GIT repository.

jserv reviewed Jan 20, 2020

View reviewed changes

emu-rv32i.c

/* CPU state */

uint32_t pc;

uint32_t next_pc;

uint32_t insn;

uint32_t reg[32];

Copy link

Contributor

jserv Jan 20, 2020

Choose a reason for hiding this comment

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

Ditto. Minimize the changes.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the compressed instructions #5

Implement the compressed instructions #5

eriktai commented Jan 11, 2020

jserv commented Jan 11, 2020

jserv Jan 11, 2020

jserv Jan 12, 2020

eriktai Jan 12, 2020

jserv commented Jan 11, 2020

eriktai commented Jan 11, 2020 •

edited

Loading

jserv commented Jan 11, 2020

eriktai commented Jan 11, 2020

jserv commented Jan 11, 2020

jserv Jan 11, 2020

jserv commented Jan 11, 2020

jserv commented Jan 12, 2020

jserv commented Jan 12, 2020

jserv commented Jan 12, 2020

jserv Jan 12, 2020

jserv Jan 12, 2020

jserv Jan 12, 2020

jserv Jan 12, 2020

jserv Jan 12, 2020

eriktai Jan 12, 2020

jserv Jan 12, 2020

jserv commented Jan 12, 2020

jserv commented Jan 16, 2020

eriktai commented Jan 19, 2020 via email

eriktai commented Jan 20, 2020

jserv Jan 20, 2020

jserv Jan 20, 2020

jserv Jan 20, 2020

jserv Jan 20, 2020

jserv commented Jan 20, 2020

Implement the compressed instructions #5

Are you sure you want to change the base?

Implement the compressed instructions #5

Conversation

eriktai commented Jan 11, 2020

Some modification of rv32emu to support C ext.

Future work

jserv commented Jan 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jserv commented Jan 11, 2020

eriktai commented Jan 11, 2020 • edited Loading

jserv commented Jan 11, 2020

eriktai commented Jan 11, 2020

jserv commented Jan 11, 2020

Choose a reason for hiding this comment

jserv commented Jan 11, 2020

jserv commented Jan 12, 2020

jserv commented Jan 12, 2020

jserv commented Jan 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jserv commented Jan 12, 2020

jserv commented Jan 16, 2020

eriktai commented Jan 19, 2020 via email

eriktai commented Jan 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jserv commented Jan 20, 2020

eriktai commented Jan 11, 2020 •

edited

Loading