RE: [PATCH] disas/nanomips: Convert nanoMIPS disassembler to C

[Vince Del Vecchio]

On Fri, 29 Jul 2022 at 10:18, Peter Maydell <peter.maydell@linaro.org> wrote:
> ...
> Is it possible to break this down into smaller pieces so it isn't one single 
> enormous 5000 line patch ?
> 
> I guess partial conversion is likely to run into compilation difficulties 
> mid-series; if so we could do "disable the disassembler; convert it; reenable 
> it".
> 
> The rationale here is code review -- reviewing a single huge patch is 
> essentially impossible, so it needs to be broken down into coherent smaller 
> pieces to be reviewable.

Hi Peter,

Something like 90% of the patch is purely mechanical transformations of which 
I've excerpted two examples below.  (There are 3-4 chunks of mechanical 
transformations, of which I've shown the most complicated type that accounts 
for ~60% of the changed lines.)  Did you intend to review this by having a 
human inspect all of these?

Would it be feasible instead to provide a sed script to effect the mechanical 
parts of the patch (the reviewer could review the script, apply it themselves 
to verify equivalence if desired, and spot check the result) together with a 
(much smaller) non-mechanical diff?

-Vince


@@ -2004,17 +1740,17 @@ std::string NMD::ADD_S(uint64 instruction)
  *     rt -----
  *          rs -----
  */
-std::string NMD::ADDIU_32_(uint64 instruction)
+static char *ADDIU_32_(uint64 instruction)
 {
     uint64 rt_value = extract_rt_25_24_23_22_21(instruction);
     uint64 rs_value = extract_rs_20_19_18_17_16(instruction);
     uint64 u_value = extract_u_15_to_0(instruction);
 
-    std::string rt = GPR(copy(rt_value));
-    std::string rs = GPR(copy(rs_value));
-    std::string u = IMMEDIATE(copy(u_value));
+    const char *rt = GPR(copy_ui(rt_value));
+    const char *rs = GPR(copy_ui(rs_value));
+    const char *u = IMMEDIATE_UI(copy_ui(u_value));
 
-    return img::format("ADDIU %s, %s, %s", rt, rs, u);
+    return img_format("ADDIU %s, %s, %s", rt, rs, u);
 }
 
 
@@ -2027,15 +1763,15 @@ std::string NMD::ADDIU_32_(uint64 instruction)
  *     rt -----
  *          rs -----
  */
-std::string NMD::ADDIU_48_(uint64 instruction)
+static char *ADDIU_48_(uint64 instruction)
 {
     uint64 rt_value = extract_rt_41_40_39_38_37(instruction);
     int64 s_value = extract_s__se31_15_to_0_31_to_16(instruction);
 
-    std::string rt = GPR(copy(rt_value));
-    std::string s = IMMEDIATE(copy(s_value));
+    const char *rt = GPR(copy_ui(rt_value));
+    const char *s = IMMEDIATE_I(copy_i(s_value));
 
-    return img::format("ADDIU %s, %s", rt, s);
+    return img_format("ADDIU %s, %s", rt, s);
 }