Merge pull request #253 from water111/w/constants-in-conditions

[Decompiler] Fixes for gkernel and pad

common/versions.h

@@ -12,7 +12,7 @@ namespace versions {
 constexpr s32 GOAL_VERSION_MAJOR = 0;
 constexpr s32 GOAL_VERSION_MINOR = 6;
 
-constexpr int DECOMPILER_VERSION = 1;
+constexpr int DECOMPILER_VERSION = 2;
 
 // these versions are from the game
 constexpr u32 ART_FILE_VERSION = 6;

decompiler/IR2/AtomicOpForm.cpp

@@ -153,6 +153,38 @@ FormElement* StoreOp::get_as_form(FormPool& pool, const Env& env) const {
         }
       }
 
+      if (input_type.kind == TP_Type::Kind::OBJECT_PLUS_PRODUCT_WITH_CONSTANT) {
+        FieldReverseLookupInput rd_in;
+        DerefKind dk;
+        dk.is_store = true;
+        dk.reg_kind = get_reg_kind(ro.reg);
+        dk.size = m_size;
+        rd_in.deref = dk;
+        rd_in.base_type = input_type.get_obj_plus_const_mult_typespec();
+        rd_in.stride = input_type.get_multiplier();
+        rd_in.offset = ro.offset;
+        auto rd = env.dts->ts.reverse_field_lookup(rd_in);
+
+        if (rd.success) {
+          std::vector<DerefToken> tokens;
+          assert(!rd.tokens.empty());
+          for (auto& token : rd.tokens) {
+            tokens.push_back(to_token(token));
+          }
+
+          // we pass along the register offset because code generation seems to be a bit
+          // different in different cases.
+          auto source = pool.alloc_single_element_form<ArrayFieldAccess>(
+              nullptr, ro.var, tokens, input_type.get_multiplier(), ro.offset);
+
+          auto val = pool.alloc_single_element_form<SimpleExpressionElement>(
+              nullptr, m_value.as_expr(), m_my_idx);
+
+          assert(!rd.addr_of);
+          return pool.alloc_element<SetFormFormElement>(source, val);
+        }
+      }
+
       FieldReverseLookupInput rd_in;
       DerefKind dk;
       dk.is_store = true;

decompiler/IR2/Form.cpp

@@ -274,12 +274,16 @@ void SimpleAtomElement::get_modified_regs(RegSet& regs) const {
 // SetVarElement
 /////////////////////////////
 
-SetVarElement::SetVarElement(const Variable& var, Form* value, bool is_sequence_point)
-    : m_dst(var), m_src(value), m_is_sequence_point(is_sequence_point) {
+SetVarElement::SetVarElement(const Variable& var,
+                             Form* value,
+                             bool is_sequence_point,
+                             const SetVarInfo& info)
+    : m_dst(var), m_src(value), m_is_sequence_point(is_sequence_point), m_var_info(info) {
   value->parent_element = this;
 }
 
 goos::Object SetVarElement::to_form_internal(const Env& env) const {
+  assert(active());
   return pretty_print::build_list("set!", m_dst.to_form(env), m_src->to_form(env));
 }
 

decompiler/IR2/Form.h

@@ -223,7 +223,10 @@ class SimpleAtomElement : public FormElement {
  */
 class SetVarElement : public FormElement {
  public:
-  SetVarElement(const Variable& var, Form* value, bool is_sequence_point);
+  SetVarElement(const Variable& var,
+                Form* value,
+                bool is_sequence_point,
+                const SetVarInfo& info = {});
   goos::Object to_form_internal(const Env& env) const override;
   void apply(const std::function<void(FormElement*)>& f) override;
   void apply_form(const std::function<void(Form*)>& f) override;
@@ -241,28 +244,23 @@ class SetVarElement : public FormElement {
   const Variable& dst() const { return m_dst; }
   const Form* src() const { return m_src; }
   Form* src() { return m_src; }
-  bool is_eliminated_coloring_move() const { return m_is_eliminated_coloring_move; }
-  void eliminate_as_coloring_move() { m_is_eliminated_coloring_move = true; }
+  bool is_eliminated_coloring_move() const { return m_var_info.is_eliminated_coloring_move; }
+  void eliminate_as_coloring_move() { m_var_info.is_eliminated_coloring_move = true; }
 
-  bool is_dead_set() const { return m_is_dead_set; }
-  void mark_as_dead_set() { m_is_dead_set = true; }
+  bool is_dead_set() const { return m_var_info.is_dead_set; }
+  void mark_as_dead_set() { m_var_info.is_dead_set = true; }
 
-  bool is_dead_false_set() const { return m_is_dead_false; }
-  void mark_as_dead_false() { m_is_dead_false = true; }
+  bool is_dead_false_set() const { return m_var_info.is_dead_false; }
+  void mark_as_dead_false() { m_var_info.is_dead_false = true; }
+
+  const SetVarInfo& info() const { return m_var_info; }
 
  private:
   Variable m_dst;
   Form* m_src = nullptr;
   bool m_is_sequence_point = true;
 
-  // is this a compiler-inserted move at the beginning of a function
-  // that should be eliminated?
-  bool m_is_eliminated_coloring_move = false;
-  // is this a (set! var expr) which consumes the reg for expr,
-  // and var is written and unused?
-  bool m_is_dead_set = false;
-  // is this a (set! var #f) where the value of #f isn't used?
-  bool m_is_dead_false = false;
+  SetVarInfo m_var_info;
 };
 
 /*!

decompiler/IR2/FormExpressionAnalysis.cpp

@@ -746,14 +746,14 @@ void SetVarElement::push_to_stack(const Env& env, FormPool& pool, FormStack& sta
         auto& info = env.reg_use().op.at(var.idx());
         if (info.consumes.find(var.reg()) != info.consumes.end()) {
           stack.push_non_seq_reg_to_reg(m_dst, src_as_se->expr().get_arg(0).var(), m_src,
-                                        m_is_eliminated_coloring_move);
+                                        m_var_info);
           return;
         }
       }
     }
   }
 
-  stack.push_value_to_reg(m_dst, m_src, true, m_is_eliminated_coloring_move);
+  stack.push_value_to_reg(m_dst, m_src, true, m_var_info);
   for (auto x : m_src->elts()) {
     assert(x->parent_form == m_src);
   }
@@ -773,8 +773,8 @@ void SetVarElement::update_from_stack(const Env& env,
 
 void SetFormFormElement::push_to_stack(const Env& env, FormPool& pool, FormStack& stack) {
   // todo - is the order here right?
-  m_src->update_children_from_stack(env, pool, stack, false);
   m_dst->update_children_from_stack(env, pool, stack, false);
+  m_src->update_children_from_stack(env, pool, stack, false);
   stack.push_form_element(this, true);
 }
 
@@ -1141,7 +1141,7 @@ void CondWithElseElement::push_to_stack(const Env& env, FormPool& pool, FormStac
       }
 
       std::vector<FormElement*> new_entries;
-      if (form == entry.body && rewrite_as_set) {
+      if (form == entry.body && rewrite_as_set && !set_unused) {
         new_entries = temp_stack.rewrite_to_get_var(pool, *last_var, env);
       } else {
         new_entries = temp_stack.rewrite(pool);
@@ -1161,7 +1161,7 @@ void CondWithElseElement::push_to_stack(const Env& env, FormPool& pool, FormStac
   }
 
   std::vector<FormElement*> new_entries;
-  if (rewrite_as_set) {
+  if (rewrite_as_set && !set_unused) {
     new_entries = temp_stack.rewrite_to_get_var(pool, *last_var, env);
   } else {
     new_entries = temp_stack.rewrite(pool);
@@ -1405,23 +1405,40 @@ FormElement* ConditionElement::make_generic(const Env&,
 }
 
 void ConditionElement::push_to_stack(const Env& env, FormPool& pool, FormStack& stack) {
-  std::vector<Form*> source_forms;
+  std::vector<Form*> source_forms, popped_forms;
   std::vector<TypeSpec> source_types;
   std::vector<Variable> vars;
 
   for (int i = 0; i < get_condition_num_args(m_kind); i++) {
-    auto& var = m_src[i]->var();
-    vars.push_back(var);
-    source_types.push_back(env.get_types_before_op(var.idx()).get(var.reg()).typespec());
+    if (m_src[i]->is_var()) {
+      auto& var = m_src[i]->var();
+      vars.push_back(var);
+      source_types.push_back(env.get_types_before_op(var.idx()).get(var.reg()).typespec());
+    } else if (m_src[i]->is_int()) {
+      source_types.push_back(TypeSpec("int"));
+    } else {
+      throw std::runtime_error("Unsupported atom in ConditionElement::push_to_stack");
+    }
   }
   if (m_flipped) {
     std::reverse(vars.begin(), vars.end());
   }
 
-  source_forms = pop_to_forms(vars, env, pool, stack, true, m_consumed);
+  popped_forms = pop_to_forms(vars, env, pool, stack, true, m_consumed);
   if (m_flipped) {
-    std::reverse(source_forms.begin(), source_forms.end());
+    std::reverse(popped_forms.begin(), popped_forms.end());
+  }
+
+  int popped_counter = 0;
+  for (int i = 0; i < get_condition_num_args(m_kind); i++) {
+    if (m_src[i]->is_var()) {
+      source_forms.push_back(popped_forms.at(popped_counter++));
+    } else {
+      source_forms.push_back(pool.alloc_single_element_form<SimpleAtomElement>(nullptr, *m_src[i]));
+    }
   }
+  assert(popped_counter == int(popped_forms.size()));
+  assert(source_forms.size() == source_types.size());
 
   stack.push_form_element(make_generic(env, pool, source_forms, source_types), true);
 }

decompiler/IR2/FormStack.cpp

@@ -25,29 +25,32 @@ std::string FormStack::print(const Env& env) {
   return result;
 }
 
-void FormStack::push_value_to_reg(Variable var, Form* value, bool sequence_point, bool is_elim) {
+void FormStack::push_value_to_reg(Variable var,
+                                  Form* value,
+                                  bool sequence_point,
+                                  const SetVarInfo& info) {
   assert(value);
   StackEntry entry;
   entry.active = true;  // by default, we should display everything!
   entry.sequence_point = sequence_point;
   entry.destination = var;
   entry.source = value;
-  entry.eliminated_as_coloring_move = is_elim;
+  entry.set_info = info;
   m_stack.push_back(entry);
 }
 
 void FormStack::push_non_seq_reg_to_reg(const Variable& dst,
                                         const Variable& src,
                                         Form* src_as_form,
-                                        bool is_elim) {
+                                        const SetVarInfo& info) {
   assert(src_as_form);
   StackEntry entry;
   entry.active = true;
   entry.sequence_point = false;
   entry.destination = dst;
   entry.non_seq_source = src;
   entry.source = src_as_form;
-  entry.eliminated_as_coloring_move = is_elim;
+  entry.set_info = info;
   m_stack.push_back(entry);
 }
 
@@ -152,10 +155,8 @@ std::vector<FormElement*> FormStack::rewrite(FormPool& pool) {
     }
 
     if (e.destination.has_value()) {
-      auto elt = pool.alloc_element<SetVarElement>(*e.destination, e.source, e.sequence_point);
-      if (e.eliminated_as_coloring_move) {
-        elt->eliminate_as_coloring_move();
-      }
+      auto elt =
+          pool.alloc_element<SetVarElement>(*e.destination, e.source, e.sequence_point, e.set_info);
       e.source->parent_element = elt;
       result.push_back(elt);
     } else {

decompiler/IR2/FormStack.h

@@ -16,11 +16,11 @@ class FormStack {
   void push_value_to_reg(Variable var,
                          Form* value,
                          bool sequence_point,
-                         bool is_eliminated = false);
+                         const SetVarInfo& info = {});
   void push_non_seq_reg_to_reg(const Variable& dst,
                                const Variable& src,
                                Form* src_as_form,
-                               bool is_elim);
+                               const SetVarInfo& info = {});
   void push_form_element(FormElement* elt, bool sequence_point);
   Form* pop_reg(const Variable& var,
                 const RegSet& barrier,
@@ -38,11 +38,13 @@ class FormStack {
     std::optional<Variable> destination;  // what register we are setting (or nullopt if no dest.)
     std::optional<Variable> non_seq_source;  // source variable, if we are setting var to var.
     Form* source = nullptr;                  // the value we are setting the register to.
-    bool eliminated_as_coloring_move = false;
 
     FormElement* elt = nullptr;
     bool sequence_point = false;
     TP_Type type;
+
+    SetVarInfo set_info;
+
     std::string print(const Env& env) const;
   };
   std::vector<StackEntry> m_stack;

decompiler/IR2/IR2_common.h

@@ -149,4 +149,16 @@ struct VariableNames {
     }
   }
 };
+
+struct SetVarInfo {
+  // is this a compiler-inserted move at the beginning of a function
+  // that should be eliminated?
+  bool is_eliminated_coloring_move = false;
+  // is this a (set! var expr) which consumes the reg for expr,
+  // and var is written and unused?
+  bool is_dead_set = false;
+  // is this a (set! var #f) where the value of #f isn't used?
+  bool is_dead_false = false;
+};
+
 }  // namespace decompiler

decompiler/config/all-types.gc

@@ -2031,7 +2031,7 @@
    (state              int32      :offset-assert 84)
    (align              uint8 6  :offset-assert 88)
    (direct             uint8 6  :offset-assert 94)
-   (buzz-val           uint16 2  :offset-assert 100)
+   (buzz-val           uint8 2  :offset-assert 100)
    (buzz-time          uint64 2  :offset-assert 104)
    (buzz               basic      :offset-assert 120)
    (buzz-act           int32      :offset-assert 124)

doc/decompiler_changelog.md

@@ -6,4 +6,10 @@
 - Remove useless `(set! <a> <b>)` and eliminate useless temporaries created by these.
 - Remove useless `set!`s sometimes appearing around functions `(set! <a> (some-function ...))` when the result is unused, but moved into a different register.
 - Recognize `(break!)` (GOAL breakpoint)
-- Support `(new 'process ...)`
+- Support `(new 'process ...)`
+
+## Version 2
+- Expressions like `(set! (-> a b) (-> c d))` are much less likely to have fake temporaries.
+- Many more useless `set!`s will be removed
+- Stores into arrays are supported
+- Fixed bug where unused/eliminated temporaries would sometimes be used as the result of a block