Support static analysis of registers #192

ChinYikMing · 2023-08-14T14:53:27Z

Introduce reg_used_mask in rv_insn_t structure, so that we can know which registers are used during the static analysis of the instruction.

Also, refactor the initialization of enumeration of registers with X-macro for reusing the macro in the tool

src/riscv.h

jserv · 2023-08-14T17:15:48Z

I am hesitant to embrace the proposed changes due to the alteration in the instruction decoding structure. I require some numerical data to illustrate that the impact on overall performance is negligible.

src/riscv.h

src/riscv_private.h

jserv · 2023-08-15T01:00:02Z

By the way, could you verify the percentage displayed in rv_histogram? For example,

  1. addi       0.29% [6662      ] █████████████████████████████████████████████
  2. sw         0.13% [2932      ] ███████████████████▊
  3. lw         0.12% [2806      ] ██████████████████▉
  4. jal        0.08% [1759      ] ███████████▉
  5. beq        0.04% [928       ] ██████▎
  6. bne        0.04% [885       ] █████▉
  7. add        0.04% [836       ] █████▋
  8. slli       0.02% [560       ] ███▊
  9. andi       0.02% [522       ] ███▌
 10. lui        0.02% [469       ] ███▏
 11. sub        0.02% [463       ] ███▏
 12. srli       0.02% [437       ] ██▉
 13. or         0.02% [425       ] ██▊
 14. jalr       0.01% [342       ] ██▎
 15. bge        0.01% [303       ] ██
 16. lhu        0.01% [300       ] ██
 17. sh         0.01% [261       ] █▊
 18. blt        0.01% [243       ] █▋
 19. lbu        0.01% [239       ] █▌
 20. and        0.01% [239       ] █▌

Having 0.29% as the top portion can be confusing.

ChinYikMing · 2023-08-15T04:40:30Z

I am hesitant to embrace the proposed changes due to the alteration in the instruction decoding structure. I require some numerical data to illustrate that the impact on overall performance is negligible.

Are there any integrated benchmarks can provide the numerical data?

jserv · 2023-08-15T04:43:23Z

Is there any integrated benchmarks can provide the numerical data?

Check prebuilt CoreMark, Dhrystone, nqueens, SciMar2, and richards in build directory.

ChinYikMing · 2023-08-15T04:47:49Z

By the way, could you verify the percentage displayed in rv_histogram? For example,

  1. addi       0.29% [6662      ] █████████████████████████████████████████████
  2. sw         0.13% [2932      ] ███████████████████▊
  3. lw         0.12% [2806      ] ██████████████████▉
  4. jal        0.08% [1759      ] ███████████▉
  5. beq        0.04% [928       ] ██████▎
  6. bne        0.04% [885       ] █████▉
  7. add        0.04% [836       ] █████▋
  8. slli       0.02% [560       ] ███▊
  9. andi       0.02% [522       ] ███▌
 10. lui        0.02% [469       ] ███▏
 11. sub        0.02% [463       ] ███▏
 12. srli       0.02% [437       ] ██▉
 13. or         0.02% [425       ] ██▊
 14. jalr       0.01% [342       ] ██▎
 15. bge        0.01% [303       ] ██
 16. lhu        0.01% [300       ] ██
 17. sh         0.01% [261       ] █▊
 18. blt        0.01% [243       ] █▋
 19. lbu        0.01% [239       ] █▌
 20. and        0.01% [239       ] █▌

Having 0.29% as the top portion can be confusing.

Sorry, I forgot convert to percentage

jserv · 2023-08-15T04:52:15Z

Sorry, I forgot convert to percentage

Decouple the percentage fix in another pull request since this pull request is blocked for the sake of further measurements.

ChinYikMing · 2023-08-15T06:09:38Z

I am hesitant to embrace the proposed changes due to the alteration in the instruction decoding structure. I require some numerical data to illustrate that the impact on overall performance is negligible.

I have run 10 times of CoreMark and Dhrystone respectively and calculated their average. The performance lost is around 0.6% on CoreMark and 0.5% on Dhrystone.

Before changes average:

CoreMark ( Instructions/Sec )	Dhrystone ( MIPS )
1087.843	1268.44

After changes average:

CoreMark ( Instructions/Sec )	Dhrystone ( MIPS )
1080.594	1261.88

tools/rv_histogram.c

qwe661234 · 2023-08-15T08:12:13Z

Issue reproducer

./build/rv_histogram -a ./build/dhrystone.elf

+---------------------------------------------+
| RV32 Target Instruction Frequency Histogram |
+---------------------------------------------+
106. lbu        0.01% [221       ] ███▍
107. sh         0.01% [227       ] ███▌
108. blt        0.01% [232       ] ███▋
109. and        0.01% [240       ] ███▊
110. lhu        0.01% [276       ] ████▎
111. jalr       0.01% [294       ] ████▋
112. bge        0.01% [296       ] ████▋
113. srli       0.02% [399       ] ██████▎
114. or         0.02% [409       ] ██████▍
115. sub        0.02% [441       ] ██████▉
116. lui        0.02% [442       ] ██████▉
117. andi       0.02% [446       ] ███████
118. slli       0.02% [484       ] ███████▋
119. add        0.03% [745       ] ███████████▋
120. beq        0.04% [790       ] ████████████▍
121. bne        0.04% [814       ] ████████████▊
122. jal        0.08% [1759      ] ███████████████████████████▋
123. lw         0.12% [2641      ] █████████████████████████████████████████▋
124. sw         0.13% [2837      ] ████████████████████████████████████████████▋
125. addi       0.30% [6469      ] ██████████████████████████████████████████████████████████████████████████████████████████████████████

Issue

The table number begin from 106

src/decode.h

ChinYikMing · 2023-08-15T10:33:59Z

Issue reproducer

./build/rv_histogram -a ./build/dhrystone.elf

+---------------------------------------------+
| RV32 Target Instruction Frequency Histogram |
+---------------------------------------------+
106. lbu        0.01% [221       ] ███▍
107. sh         0.01% [227       ] ███▌
108. blt        0.01% [232       ] ███▋
109. and        0.01% [240       ] ███▊
110. lhu        0.01% [276       ] ████▎
111. jalr       0.01% [294       ] ████▋
112. bge        0.01% [296       ] ████▋
113. srli       0.02% [399       ] ██████▎
114. or         0.02% [409       ] ██████▍
115. sub        0.02% [441       ] ██████▉
116. lui        0.02% [442       ] ██████▉
117. andi       0.02% [446       ] ███████
118. slli       0.02% [484       ] ███████▋
119. add        0.03% [745       ] ███████████▋
120. beq        0.04% [790       ] ████████████▍
121. bne        0.04% [814       ] ████████████▊
122. jal        0.08% [1759      ] ███████████████████████████▋
123. lw         0.12% [2641      ] █████████████████████████████████████████▋
124. sw         0.13% [2837      ] ████████████████████████████████████████████▋
125. addi       0.30% [6469      ] ██████████████████████████████████████████████████████████████████████████████████████████████████████

Issue

The table number begin from 106

fixed it

src/decode.h

jserv · 2023-08-15T12:48:24Z

Consider the following changes:

diff --git a/src/common.h b/src/common.h
index faa2f6d..5634734 100644
--- a/src/common.h
+++ b/src/common.h
@@ -67,6 +67,20 @@
 /* run the 1st parameter */
 #define IIF_1(t, ...) t
 
+/* Accept any number of args >= N, but expand to just the Nth one. The macro
+ * that calls the function still only supports 4 args, but the set of values
+ * that might need to be returned is 1 larger, so N is increased to 6.
+ */
+#define _GET_NTH_ARG(_1, _2, _3, _4, _5, N, ...) N
+
+/* Count how many args are in a variadic macro. The GCC/Clang extension is used
+ * to handle the case where ... expands to nothing. A placeholder arg is added
+ * before ##VA_ARGS (its value is irrelevant but necessary to preserve the
+ * shifting offset).
+ * Additionally, 0 is added as a valid value in the N position.
+ */
+#define COUNT_VARARGS(...) _GET_NTH_ARG("ignored", ##__VA_ARGS__, 4, 3, 2, 1, 0)
+
 #if defined(__GNUC__) || defined(__clang__)
 #define __HAVE_TYPEOF 1
 #endif
diff --git a/src/decode.h b/src/decode.h
index c895166..070d3fb 100644
--- a/src/decode.h
+++ b/src/decode.h
@@ -10,12 +10,28 @@
 
 #include "riscv.h"
 
+enum op_field {
+    F_rs1 = 1,
+    F_rs2 = 2,
+    F_rs3 = 4,
+    F_rd = 8,
+};
+
+#define ENC4(a, b, c, d, ...) F_##a | F_##b | F_##c | F_##d
+#define ENC3(a, b, c, ...) F_##a | F_##b | F_##c
+#define ENC2(a, b, ...) F_##a | F_##b
+#define ENC1(a, ...) F_##a
+
+#define ENCN(X, A) X##A
+#define ENC_GEN(X, A) ENCN(X, A)
+#define ENC(...) ENC_GEN(ENC, COUNT_VARARGS(__VA_ARGS__))(__VA_ARGS__)
+
 /* RISC-V instruction list in format _(instruction-name, can-branch) */
 /* clang-format off */
 #define RISCV_INSN_LIST                    \
-    _(nop, 0, 0x1|0x8)                     \
+    _(nop, 0, ENC(rs1, rd))                \
     /* RV32I Base Instruction Set */       \
-    _(lui, 0, 0x8)                         \
+    _(lui, 0, ENC(rd))                     \
...

ChinYikMing · 2023-08-15T13:39:41Z

Consider the following changes:

diff --git a/src/common.h b/src/common.h
index faa2f6d..5634734 100644
--- a/src/common.h
+++ b/src/common.h
@@ -67,6 +67,20 @@
 /* run the 1st parameter */
 #define IIF_1(t, ...) t
 
+/* Accept any number of args >= N, but expand to just the Nth one. The macro
+ * that calls the function still only supports 4 args, but the set of values
+ * that might need to be returned is 1 larger, so N is increased to 6.
+ */
+#define _GET_NTH_ARG(_1, _2, _3, _4, _5, N, ...) N
+
+/* Count how many args are in a variadic macro. The GCC/Clang extension is used
+ * to handle the case where ... expands to nothing. A placeholder arg is added
+ * before ##VA_ARGS (its value is irrelevant but necessary to preserve the
+ * shifting offset).
+ * Additionally, 0 is added as a valid value in the N position.
+ */
+#define COUNT_VARARGS(...) _GET_NTH_ARG("ignored", ##__VA_ARGS__, 4, 3, 2, 1, 0)
+
 #if defined(__GNUC__) || defined(__clang__)
 #define __HAVE_TYPEOF 1
 #endif
diff --git a/src/decode.h b/src/decode.h
index c895166..070d3fb 100644
--- a/src/decode.h
+++ b/src/decode.h
@@ -10,12 +10,28 @@
 
 #include "riscv.h"
 
+enum op_field {
+    F_rs1 = 1,
+    F_rs2 = 2,
+    F_rs3 = 4,
+    F_rd = 8,
+};
+
+#define ENC4(a, b, c, d, ...) F_##a | F_##b | F_##c | F_##d
+#define ENC3(a, b, c, ...) F_##a | F_##b | F_##c
+#define ENC2(a, b, ...) F_##a | F_##b
+#define ENC1(a, ...) F_##a
+
+#define ENCN(X, A) X##A
+#define ENC_GEN(X, A) ENCN(X, A)
+#define ENC(...) ENC_GEN(ENC, COUNT_VARARGS(__VA_ARGS__))(__VA_ARGS__)
+
 /* RISC-V instruction list in format _(instruction-name, can-branch) */
 /* clang-format off */
 #define RISCV_INSN_LIST                    \
-    _(nop, 0, 0x1|0x8)                     \
+    _(nop, 0, ENC(rs1, rd))                \
     /* RV32I Base Instruction Set */       \
-    _(lui, 0, 0x8)                         \
+    _(lui, 0, ENC(rd))                     \
...

Nice macro manipulation! I added ENC0 for consistency

jserv · 2023-08-15T13:48:09Z

Nice macro manipulation! I added ENC0 for consistency

C macros can be powerful. I always learn from metalang99, which provides a solid foundation for writing reliable and maintainable metaprograms in pure C99."

jserv · 2023-08-15T13:52:10Z

Thank @ChinYikMing for contributing!

ChinYikMing requested a review from jserv August 14, 2023 15:01

jserv reviewed Aug 14, 2023

View reviewed changes

src/riscv.h Outdated Show resolved Hide resolved