[Decompiler] Fix let bug (#328)

* fix let definition bug

* remove debug prints and add test

decompiler/ObjectFile/ObjectFileDB_IR2.cpp

@@ -743,16 +743,16 @@ std::string ObjectFileDB::ir2_function_to_string(ObjectFileData& data, Function&
               op.reg_type_info_as_string(*init_types, func.ir2.env.get_types_after_op(op_id)), 50);
         }
 
-        if (func.ir2.env.has_reg_use()) {
+        /*if (func.ir2.env.has_reg_use()) {
           std::string regs;
-          for (auto r : func.ir2.env.reg_use().op.at(op_id).consumes) {
+          for (auto r : func.ir2.env.reg_use().op.at(op_id).live_in) {
             regs += r.to_charp();
             regs += ' ';
           }
           if (!regs.empty()) {
-            append_commented(line, printed_comment, "cs: " + regs, 50);
+            append_commented(line, printed_comment, "lvi: " + regs, 50);
           }
-        }
+        }*/
       }
       auto& instr = func.instructions.at(instr_id);
       // print linked strings

decompiler/analysis/insert_lets.cpp

@@ -344,17 +344,31 @@ LetStats insert_lets(const Function& func, Env& env, FormPool& pool, Form* top_l
     if (first_form_as_set &&
         env.get_variable_name(first_form_as_set->dst()) == env.get_variable_name(info.access) &&
         !first_form_as_set->info().is_eliminated_coloring_move) {
+      bool allowed = true;
+
+      RegAccessSet ras;
+      first_form_as_set->src()->collect_vars(ras, true);
+      for (auto ra : ras) {
+        if (ra.reg() == first_form_as_set->dst().reg()) {
+          if (env.get_variable_name(ra) == env.get_variable_name(first_form_as_set->dst())) {
+            allowed = false;
+            break;
+          }
+        }
+      }
       // success!
       // fmt::print("Want let for {} range {} to {}\n",
       // env.get_variable_name(first_form_as_set->dst()), info.start_idx, info.end_idx);
-      LetInsertion li;
-      li.form = info.lca_form;
-      li.start_elt = info.start_idx;
-      li.end_elt = info.end_idx;
-      li.set_form = first_form_as_set;
-      li.name = info.var_name;
-      possible_insertions[li.form].push_back(li);
-      stats.vars_in_lets++;
+      if (allowed) {
+        LetInsertion li;
+        li.form = info.lca_form;
+        li.start_elt = info.start_idx;
+        li.end_elt = info.end_idx;
+        li.set_form = first_form_as_set;
+        li.name = info.var_name;
+        possible_insertions[li.form].push_back(li);
+        stats.vars_in_lets++;
+      }
     } else {
       // fmt::print("fail for {} : {}\n", info.var_name, first_form->to_string(env));
     }

decompiler/analysis/reg_usage.cpp

@@ -155,6 +155,36 @@ RegUsageInfo analyze_ir2_register_usage(const Function& function) {
     phase3(*ops, blocks, i, &result);
   }
 
+  // compute live in
+  for (int block_id = 0; block_id < int(blocks.size()); block_id++) {
+    int end_op = ops->block_id_to_end_atomic_op.at(block_id);
+    int start_op = ops->block_id_to_first_atomic_op.at(block_id);
+
+    for (int instr_id = start_op + 1; instr_id < end_op; instr_id++) {
+      result.op.at(instr_id).live_in = result.op.at(instr_id - 1).live;
+    }
+
+    if (end_op > start_op) {
+      auto& last_live_out = result.op.at(end_op - 1).live;
+      for (auto succ : {blocks.at(block_id).succ_branch, blocks.at(block_id).succ_ft}) {
+        if (succ != -1) {
+          auto succ_id = ops->block_id_to_first_atomic_op.at(succ);  // todo?
+          result.op.at(succ_id).live_in.insert(last_live_out.begin(), last_live_out.end());
+        }
+      }
+    }
+  }
+
+  // special case for the very first op
+  auto& first_op_live_out = result.op.at(0).live;
+  RegSet first_op_live_in;
+  first_op_live_in.insert(first_op_live_out.begin(), first_op_live_out.end());
+  for (auto reg : ops->ops.at(0)->write_regs()) {
+    first_op_live_in.erase(reg);
+  }
+  first_op_live_in.insert(ops->ops.at(0)->read_regs().begin(), ops->ops.at(0)->read_regs().end());
+  result.op.at(0).live_in = first_op_live_in;
+
   // we want to know if an op "consumes" a register.
   // this means the value of the register coming in is:
   // A. read by the operation

decompiler/analysis/reg_usage.h

@@ -16,7 +16,7 @@ struct RegUsageInfo {
   };
 
   struct PerOp {
-    RegSet live, dead, consumes, written_and_unused;
+    RegSet live, dead, consumes, written_and_unused, live_in;
   };
 
   int block_count() const { return int(block.size()); }

test/decompiler/reference/gkernel-h_REF.gc

@@ -288,15 +288,21 @@
 ;; WARN: Unsupported inline assembly instruction kind - [5]
 ;; WARN: Unsupported inline assembly instruction kind - [73]
 (defmethod print handle ((obj handle))
-  (local-vars (a2-0 int) (s7-0 none))
+  (local-vars
+   (r0-0 none)
+   (a2-0 int)
+   (a2-1 process)
+   (a2-2 (pointer process))
+   (s7-0 none)
+   )
   (if (nonzero? obj)
    (let ((t9-0 format)
          (a0-1 #t)
          (a1-0 "#<handle :process ~A :pid ~D>")
-         (v1-0 obj)
          )
-    (.subu a2-0 (the-as handle v1-0) s7-0)
-    (let ((a2-1 (and (nonzero? a2-0) (begin
+    (let ((v1-0 obj))
+     (.subu a2-0 (the-as handle v1-0) s7-0)
+     (set! a2-1 (and (nonzero? a2-0) (begin
                                       (.sllv a2-2 (the-as handle v1-0) r0-0)
                                       (let ((a3-0 (-> a2-2 0)))
                                        (set!
@@ -311,10 +317,9 @@
                                       a2-1
                                       )
                  )
-           )
-          )
-     (t9-0 a0-1 a1-0 a2-1 (sar (the-as int obj) 32))
+      )
      )
+    (t9-0 a0-1 a1-0 a2-1 (sar (the-as int obj) 32))
     )
    (format #t "#<handle :process 0 :pid 0>")
    )

test/decompiler/test_FormExpressionBuild.cpp

@@ -2735,4 +2735,58 @@ TEST_F(FormRegressionTest, StripStripTrailingWhitespace) {
       "  #f\n"
       "  )";
   test_with_expr(func, type, expected);
+}
+
+// Let bug (github #328)
+TEST_F(FormRegressionTest, TimeToGround) {
+  std::string func =
+      "sll r0, r0, 0\n"
+      "    daddiu sp, sp, -16\n"
+      "    sd fp, 8(sp)\n"
+      "    or fp, t9, r0\n"
+
+      "    mtc1 f0, r0\n"
+      "    addiu v0, r0, 0\n"
+      "    beq r0, r0, L3\n"
+      "    sll r0, r0, 0\n"
+
+      "L2:\n"
+      "    mtc1 f1, a0\n"
+      //"    lwc1 f2, L7(fp)\n"
+      "    mtc1 f2, r0\n"
+      "    mtc1 f3, a1\n"
+      "    mul.s f2, f2, f3\n"
+      "    sub.s f1, f1, f2\n"
+      "    mfc1 a0, f1\n"
+      //"    lwc1 f1, L7(fp)\n"
+      "    mtc1 f1, r0\n"
+      "    mtc1 f2, a0\n"
+      "    mul.s f1, f1, f2\n"
+      "    add.s f0, f0, f1\n"
+      "    daddiu v0, v0, 1\n"
+
+      "L3:\n"
+      "    mtc1 f1, a2\n"
+      "    neg.s f1, f1\n"
+      "    c.lt.s f1, f0\n"
+      "    bc1t L2\n"
+      "    sll r0, r0, 0\n"
+
+      "    or v1, s7, r0 \n"
+      "    ld fp, 8(sp)\n"
+      "    jr ra\n"
+      "    daddiu sp, sp, 16";
+  std::string type = "(function float float float float)";
+  std::string expected =
+      "(let ((f0-0 0.0)\n"
+      "     (v0-0 0)\n"
+      "     )\n"
+      "  (while (< (- arg2) f0-0)\n"
+      "   (set! arg0 (- arg0 (* 0.0 arg1)))\n"
+      "   (+! f0-0 (* 0.0 arg0))\n"
+      "   (+! v0-0 1)\n"
+      "   )\n"
+      "  (the-as float v0-0)\n"
+      "  )";
+  test_with_expr(func, type, expected);
 }