[decomp] clean up type def formatting and remove all-forward-definitions.gc (#608)

* clean up

* fix test

common/type_system/Type.cpp

@@ -108,6 +108,7 @@ void Field::set_inline() {
  * Compare field definitions for equality. Used to determine if redefinition would change the type.
  */
 bool Field::operator==(const Field& other) const {
+  // we don't check the score and the do-not-decompile here.
   // clang-format off
   return m_name == other.m_name &&
          m_type == other.m_type &&
@@ -116,8 +117,7 @@ bool Field::operator==(const Field& other) const {
          m_dynamic == other.m_dynamic &&
          m_array == other.m_array &&
          m_array_size == other.m_array_size &&
-         m_alignment == other.m_alignment &&
-         m_skip_in_static_decomp == other.m_skip_in_static_decomp;
+         m_alignment == other.m_alignment;
   // clang-format on
 }
 

common/type_system/TypeSystem.cpp

@@ -151,9 +151,9 @@ void TypeSystem::forward_declare_type_as(const std::string& new_type,
       auto new_parent_it = m_types.find(parent_type);
 
       auto old_ts = TypeSpec(old_parent_it->second->get_name());
-      auto new_ts = TypeSpec(new_parent_it->second->get_name());
 
       if (old_parent_it != m_types.end() && new_parent_it != m_types.end()) {
+        auto new_ts = TypeSpec(new_parent_it->second->get_name());
         if (tc(old_ts, new_ts)) {
           // new is more specific or equal to old:
           m_forward_declared_types[new_type] = new_ts.base_type();
@@ -1505,8 +1505,14 @@ std::string TypeSystem::generate_deftype_footer(const Type* type) const {
         methods_string.push_back(' ');
       }
     }
-    methods_string.append(fmt::format(
-        ") {} {})\n    ", info.type.get_arg(info.type.arg_count() - 1).print(), info.id));
+    methods_string.append(
+        fmt::format(") {} ", info.type.get_arg(info.type.arg_count() - 1).print()));
+
+    if (info.no_virtual) {
+      methods_string.append(":no-virtual ");
+    }
+
+    methods_string.append(fmt::format("{})\n    ", info.id));
   }
 
   if (!methods_string.empty()) {
@@ -1530,6 +1536,7 @@ std::string TypeSystem::generate_deftype_for_structure(const StructureType* st)
   const std::string inline_string = ":inline";
   const std::string dynamic_string = ":dynamic";
   const std::string user_offset_string = ":offset xxx";
+  bool has_offset_assert = false;
 
   for (size_t i = st->first_unique_field_idx(); i < st->fields().size(); i++) {
     const auto& field = st->fields().at(i);
@@ -1556,11 +1563,8 @@ std::string TypeSystem::generate_deftype_for_structure(const StructureType* st)
       mods += dynamic_string.size();
     }
 
-    if (field.user_placed()) {
-      if (mods) {
-        mods++;
-      }
-      mods += user_offset_string.size();
+    if (!field.user_placed()) {
+      has_offset_assert = true;
     }
     longest_mods = std::max(longest_mods, mods);
   }
@@ -1589,17 +1593,20 @@ std::string TypeSystem::generate_deftype_for_structure(const StructureType* st)
       mods += " ";
     }
 
-    if (field.user_placed()) {
-      mods += fmt::format(":offset {:3d}", field.offset());
-    }
     result.append(mods);
+    result.append(longest_mods - int(mods.size() - 1), ' ');
 
     if (!field.user_placed()) {
-      result.append(longest_mods - int(mods.size() - 1), ' ');
       result.append(":offset-assert ");
-      result.append(std::to_string(field.offset()));
+    } else {
+      if (has_offset_assert) {
+        result.append(":offset        ");
+      } else {
+        result.append(":offset ");
+      }
     }
 
+    result.append(fmt::format("{:3d}", field.offset()));
     result.append(")\n   ");
   }
 

decompiler/ObjectFile/ObjectFileDB_IR2.cpp

@@ -410,15 +410,41 @@ void ObjectFileDB::ir2_register_usage_pass() {
       func.ir2.env.set_reg_use(analyze_ir2_register_usage(func));
 
       auto block_0_start = func.ir2.env.reg_use().block.at(0).input;
+      std::vector<Register> dep_regs;
       for (auto x : block_0_start) {
-        if (x.get_kind() == Reg::VF && x.get_vf() != 0) {
-          lg::error("Bad vf dependency on {} in {}", x.to_charp(), func.guessed_name.to_string());
-          func.warnings.bad_vf_dependency("{}", x.to_string());
+        dep_regs.push_back(x);
       }
 
-        if (x.get_kind() == Reg::SPECIAL) {
+      if (!dep_regs.empty()) {
+        std::sort(dep_regs.begin(), dep_regs.end(),
+                  [](const Register& a, const Register& b) { return a.reg_id() < b.reg_id(); });
+
+        int end_valid_argument = Register(Reg::GPR, Reg::T3).reg_id() + 1;
+        if (func.type.arg_count() > 0) {
+          // end_valid_argument = Register::get_arg_reg(func.type.arg_count() - 1).reg_id();
+          end_valid_argument = Register(Reg::GPR, Reg::A0).reg_id() + func.type.arg_count() - 1;
+        }
+
+        for (auto& x : dep_regs) {
+          if ((x.get_kind() == Reg::VF && x.get_vf() != 0) || x.get_kind() == Reg::SPECIAL) {
             lg::error("Bad vf dependency on {} in {}", x.to_charp(), func.guessed_name.to_string());
             func.warnings.bad_vf_dependency("{}", x.to_string());
+            continue;
+          }
+
+          if (x == Register(Reg::GPR, Reg::S6) || x == Register(Reg::GPR, Reg::S7) ||
+              x == Register(Reg::GPR, Reg::SP) || x == Register(Reg::VF, 0)) {
+            continue;
+          }
+
+          if (x.reg_id() < end_valid_argument) {
+            continue;
+          }
+
+          lg::error("Bad register dependency on {} in {}", x.to_charp(),
+                    func.guessed_name.to_string());
+          func.warnings.general_warning("Function may read a register that is not set: {}",
+                                        x.to_string());
         }
       }
     }

decompiler/util/DecompilerTypeSystem.cpp

@@ -387,14 +387,23 @@ int DecompilerTypeSystem::get_format_arg_count(const std::string& str) const {
   if (str == "ERROR: dma tag has data in reserved bits ~X~%") {
     return 0;
   }
+
+  if (str == "#<surface f0:~m f1:~f tf+:~f tf-:~f sf:~f tvv:~m") {
+    return 5;
+  }
   int arg_count = 0;
   for (size_t i = 0; i < str.length(); i++) {
     if (str.at(i) == '~') {
       i++;  // also eat the next character.
-      if (i < str.length() && (str.at(i) == '%' || str.at(i) == 'T' || str.at(i) == '0')) {
+      if (i < str.length() && (str.at(i) == '%' || str.at(i) == 'T')) {
         // newline (~%) or tab (~T) don't take an argument.
         continue;
       }
+
+      // ~3L, ~0L do'nt seem to take arguments either.
+      if (i + 1 < str.length() && (str.at(i) == '0' || str.at(i) == '3') && str.at(i + 1) == 'L') {
+        continue;
+      }
       arg_count++;
     }
   }

test/decompiler/reference/decompiler-macros.gc

@@ -0,0 +1,81 @@
+;; This file should contain an implementation for all macros that the decompiler uses in its output.
+
+(defun ash ((value int) (shift-amount int))
+  "Arithmetic shift value by shift-amount.
+  A positive shift-amount will shift to the left and a negative will shift to the right.
+  "
+  ;; OpenGOAL does not support ash in the compiler, so we implement it here as an inline function.
+  (declare (inline))
+  (if (> shift-amount 0)
+      (shl value shift-amount)
+      (sar value (- shift-amount))
+      )
+  )
+
+(defmacro suspend()
+  '(none)
+  )
+
+(defmacro empty-form ()
+  '(none)
+  )
+
+(defmacro .sync.l ()
+  `(none))
+
+(defmacro make-u128 (upper lower)
+  `(rlet ((result :class i128)
+          (upper-xmm :class i128)
+          (lower-xmm :class i128))
+     (.mov upper-xmm ,upper)
+     (.mov lower-xmm ,lower)
+     (.pcpyld result upper-xmm lower-xmm)
+     (the uint result)
+     )
+  )
+
+(defmacro init-vf0-vector ()
+  "Initializes the VF0 vector which is a constant vector in the VU set to <0,0,0,1>"
+  `(.lvf vf0 (new 'static 'vector :x 0.0 :y 0.0 :z 0.0 :w 1.0))
+  )
+
+(defconstant SYM_TO_STRING_OFFSET #xff38)
+(defmacro symbol->string (sym)
+  "Convert a symbol to a goal string."
+  `(-> (the-as (pointer string) (+ SYM_TO_STRING_OFFSET (the-as int ,sym))))
+  )
+
+(defmacro new-stack-matrix0 ()
+  "Get a new matrix on the stack that's set to zero."
+  `(let ((mat (new 'stack-no-clear 'matrix)))
+     (set! (-> mat quad 0) (the-as uint128 0))
+     (set! (-> mat quad 1) (the-as uint128 0))
+     (set! (-> mat quad 2) (the-as uint128 0))
+     (set! (-> mat quad 3) (the-as uint128 0))
+     mat
+     )
+  )
+
+(defmacro new-stack-vector0 ()
+  "Get a stack vector that's set to 0.
+   This is more efficient than (new 'stack 'vector) because
+   this doesn't call the constructor."
+  `(let ((vec (new 'stack-no-clear 'vector)))
+     (set! (-> vec quad) (the-as uint128 0))
+     vec
+     )
+  )
+
+(defmacro with-pp (&rest body)
+  `(rlet ((pp :reg r13 :reset-here #t :type process))
+         ,@body)
+  )
+
+(defmacro fabs (x)
+  `(if (< (the float ,x) 0)
+       (- (the float ,x))
+       (the float ,x))
+  )
+
+(defconstant PI (the-as float #x40490fda))
+(defconstant MINUS_PI (the-as float #xc0490fda))

test/decompiler/reference/engine/collide/collide-cache-h_REF.gc

@@ -304,7 +304,3 @@
 
 ;; definition (perm) for symbol *collide-list*, type collide-list
 (define-perm *collide-list* collide-list (new 'global 'collide-list))
-
-
-
-

test/decompiler/reference/engine/collide/collide-mesh-h_REF.gc

@@ -147,7 +147,3 @@
 
 ;; failed to figure out what this is:
 (set! (-> *collide-mesh-cache* max-size) (the-as uint #xa000))
-
-
-
-

test/decompiler/reference/engine/collide/collide-target-h_REF.gc

@@ -72,7 +72,3 @@
   (set! (-> obj pat) (-> cshape cur-pat))
   obj
   )
-
-
-
-

test/decompiler/reference/engine/collide/collide-touch-h_REF.gc

@@ -220,7 +220,3 @@
 
 ;; definition (perm) for symbol *touching-list*, type touching-list
 (define-perm *touching-list* touching-list (new 'global 'touching-list))
-
-
-
-

test/decompiler/reference/engine/data/res-h_REF.gc

@@ -30,17 +30,17 @@
   :flag-assert         #x1600000020
   (:methods
     (new (symbol type int int) _type_ 0)
-    (get-property-data (_type_ symbol symbol float pointer (pointer res-tag) pointer) pointer 9)
-    (get-property-struct (_type_ symbol symbol float structure (pointer res-tag) pointer) structure 10)
-    (get-property-value (_type_ symbol symbol float uint128 (pointer res-tag) pointer) uint128 11)
-    (get-property-value-float (_type_ symbol symbol float float (pointer res-tag) pointer) float 12)
+    (get-property-data (_type_ symbol symbol float pointer (pointer res-tag) pointer) pointer :no-virtual 9)
+    (get-property-struct (_type_ symbol symbol float structure (pointer res-tag) pointer) structure :no-virtual 10)
+    (get-property-value (_type_ symbol symbol float uint128 (pointer res-tag) pointer) uint128 :no-virtual 11)
+    (get-property-value-float (_type_ symbol symbol float float (pointer res-tag) pointer) float :no-virtual 12)
     (get-tag-index-data (_type_ int) pointer 13)
     (get-tag-data (_type_ res-tag) pointer 14)
     (dummy-15 (_type_ res-tag) res-tag 15)
     (sort! (_type_) _type_ 16)
     (dummy-17 (_type_ res-tag pointer) res-lump 17)
     (dummy-18 (_type_ res-tag res-tag) res-lump 18)
-    (lookup-tag-idx (_type_ symbol symbol float) res-tag-pair 19)
+    (lookup-tag-idx (_type_ symbol symbol float) res-tag-pair :no-virtual 19)
     (make-property-data (_type_ float res-tag-pair pointer) pointer 20)
     (get-curve-data! (_type_ curve symbol symbol float) symbol 21)
     )

test/decompiler/reference/engine/debug/assert-h_REF.gc

@@ -64,7 +64,3 @@
 ;; failed to figure out what this is:
 (let ((v0-4 0))
   )
-
-
-
-

test/decompiler/reference/engine/dma/dma-h_REF.gc

@@ -333,8 +333,3 @@
 
 ;; definition for function dma-count-until-done
 ;; ERROR: function was not converted to expressions. Cannot decompile.
-
-
-
-
-

test/decompiler/reference/engine/draw/drawable-ambient-h_REF.gc

@@ -113,7 +113,3 @@
 ;; failed to figure out what this is:
 (let ((v0-10 0))
   )
-
-
-
-

test/decompiler/reference/engine/game/game-info-h_REF.gc

@@ -299,7 +299,3 @@
            gp-0
            )
   )
-
-
-
-

test/decompiler/reference/engine/game/settings-h_REF.gc

@@ -220,7 +220,3 @@
 ;; failed to figure out what this is:
 (let ((v0-7 0))
   )
-
-
-
-

test/decompiler/reference/engine/math/matrix_REF.gc

@@ -255,6 +255,7 @@
   )
 
 ;; definition for function matrix-transpose!
+;; WARN: Function may read a register that is not set: f31
 ;; Used lq/sq
 (defun matrix-transpose! ((dst matrix) (src matrix))
   (local-vars
@@ -337,9 +338,9 @@
   )
 
 ;; definition for function matrix-4x4-inverse!
-;; WARN: Bad vector register dependency: vf5
 ;; WARN: Bad vector register dependency: vf3
 ;; WARN: Bad vector register dependency: vf4
+;; WARN: Bad vector register dependency: vf5
 (defun matrix-4x4-inverse! ((dst matrix) (src matrix))
   (rlet ((acc :class vf)
          (Q :class vf)

test/decompiler/reference/engine/target/surface-h_REF.gc

@@ -100,7 +100,6 @@
 
 ;; definition for method 2 of type surface
 (defmethod print surface ((obj surface))
-  (local-vars (t3-0 none))
   (format
    #t
    "#<surface f0:~m f1:~f tf+:~f tf-:~f sf:~f tvv:~m"
@@ -109,7 +108,6 @@
    (-> obj tiltv)
    (-> obj tiltvv)
    (-> obj transv-max)
-   t3-0
    )
   (format
    #t

test/decompiler/reference/kernel/gkernel_REF.gc

@@ -1020,7 +1020,6 @@
 ;; definition for method 16 of type dead-pool-heap
 ;; INFO: Return type mismatch int vs none.
 (defmethod compact dead-pool-heap ((obj dead-pool-heap) (arg0 int))
-  (local-vars (a2-0 none))
   (let* ((s4-0 (memory-free obj))
          (v1-2 (memory-total obj))
          (f0-2 (/ (the float s4-0) (the float v1-2)))
@@ -1029,7 +1028,7 @@
     ((< f0-2 0.1)
      (set! arg0 1000)
      (if (and *debug-segment* (-> *kernel-context* low-memory-message))
-      (format *stdcon* "~3LLow Actor Memory~%~0L" a2-0)
+      (format *stdcon* "~3LLow Actor Memory~%~0L")
       )
      )
     ((< f0-2 0.2)

test/decompiler/reference/levels/common/rigid-body-h_REF.gc

@@ -104,7 +104,3 @@
 ;; failed to figure out what this is:
 (let ((v0-4 0))
   )
-
-
-
-

test/decompiler/test_gkernel_decomp.cpp

@@ -2578,7 +2578,7 @@ TEST_F(FormRegressionTest, ExprMethod16DeadPoolHeap) {
       "    ((< f0-2 (l.f L346))\n"
       "     (set! arg1 1000)\n"
       "     (if (and *debug-segment* (-> *kernel-context* low-memory-message))\n"
-      "      (format *stdcon* \"~3LLow Actor Memory~%~0L\" a2-0)\n"
+      "      (format *stdcon* \"~3LLow Actor Memory~%~0L\")\n"
       "      )\n"
       "     )\n"
       "    ((< f0-2 (l.f L347))\n"

test/offline/offline_test_main.cpp

@@ -319,7 +319,8 @@ TEST_F(OfflineDecompilation, CheckBasicDecode) {
 
 /*!
  * Not a super great test, but check that we find functions, methods, and logins.
- * This is a test of ir2_top_level_pass, which isn't tested as part of the normal decompiler tests.
+ * This is a test of ir2_top_level_pass, which isn't tested as part of the normal decompiler
+ tests.
  */
 TEST_F(OfflineDecompilation, FunctionDetect) {
   int function_count = 0;  // global functions
@@ -535,8 +536,8 @@ int line_count(const std::string& str) {
 TEST_F(OfflineDecompilation, Compile) {
   Compiler compiler;
 
-  compiler.run_front_end_on_file(
-      {"test", "decompiler", "reference", "all_forward_declarations.gc"});
+  compiler.run_front_end_on_file({"decompiler", "config", "all-types.gc"});
+  compiler.run_front_end_on_file({"test", "decompiler", "reference", "decompiler-macros.gc"});
 
   Timer timer;
   int total_lines = 0;