[Compiler] Add reset-here option to colored and constrained rlet vars (#169)

* add reset-here to clear coloring at entry to rlet

* update doc

decompiler/Function/ExpressionBuilder.cpp

@@ -25,12 +25,6 @@ bool Function::build_expression(LinkedObjectFile& file) {
     // or more complicated analysis to do as good as possible on outer begins.
     auto all_children = ir->get_all_ir(file);
     std::vector<IR_Begin*> all_begins;
-    for (auto i = all_children.size(); i-- > 0;) {
-      auto as_begin = dynamic_cast<IR_Begin*>(all_children.at(i).get());
-      if (as_begin) {
-        all_begins.push_back(as_begin);
-      }
-    }
 
     // the top level may also be a begin
     auto as_begin = dynamic_cast<IR_Begin*>(ir.get());
@@ -38,6 +32,13 @@ bool Function::build_expression(LinkedObjectFile& file) {
       all_begins.push_back(as_begin);
     }
 
+    for (auto& i : all_children) {
+      auto child_as_begin = dynamic_cast<IR_Begin*>(i.get());
+      if (child_as_begin) {
+        all_begins.push_back(child_as_begin);
+      }
+    }
+
     // turn each begin into an expression
     for (auto b : all_begins) {
       if (!expressionize_begin(b, file)) {

decompiler/ObjectFile/ObjectFileDB.cpp

@@ -1099,12 +1099,15 @@ void ObjectFileDB::analyze_expressions() {
     (void)segment_id;
     // register usage
     func.run_reg_usage();
+
+    if (func.attempted_type_analysis) {
       attempts++;
       if (func.build_expression(data.linked_data)) {
         success++;
       } else {
         func.warnings.append(";; Expression analysis failed.\n");
       }
+    }
   });
 
   spdlog::info(" {}/{} functions passed expression building ({:.2f}%)\n", success, attempts,

doc/changelog.md

@@ -89,3 +89,4 @@
 - Fixed a bug where accessing an inline basic field did not apply the 4-byte basic offset.
 - Made string/float constants go in the main segment when they are declared in the top-level segment, instead of the top-level segment. This is what GOAL seems to do (not 100% sure yet) and avoids issues where you set something to a string constant in the top-level. This avoids the possibility of memory bugs at the cost of more memory usage (likely very little additional memory).
 - Added support for boxed arrays. They can be created with `new` and indexed with `->`. The compound type `(array <elt-type>)` is used to describe an array with a given content type.
+- Added `reset-here` option for `rlet`.

doc/goal_doc.md

@@ -1165,16 +1165,16 @@ Not implemented well yet.
 
 ## `rlet`
 ```lisp
-(rlet ((var-name [:reg reg-name] [:class reg-class] [:type typespec])...)
+(rlet ((var-name [:reg reg-name] [:class reg-class] [:type typespec] [:reset-here #t|#f])...)
   body...
   )
 ```
 Create register variables. You can optionally specify a register with the `:reg` option and a register name like `rax` or `xmm3`. The initial value of the register is not set. If you don't specify a register, a GPR will be chosen for you by the coloring system and it will behave like a `let`.  If you don't specify a register, you can specify a register class (`gpr` or `xmm`) and the compiler will pick a GPR or XMM for you.
 
-If you pick a callee-saved register and use it within the coloring system, the compiler will back it up for you.
+If you pick a callee-saved register and use it within the coloring system, the compiler will back it up for you in the prologue and restore it in the epilogue.
 If you pick a special register like `rsp`, it won't be backed up.  
 
-Inside the `rlet`, all uses of `var-name` will always be in the given register.  If the variable goes dead (or is never live), the compiler may reuse the register as it wants.  The compiler may also spill the variable onto the stack.  Of course, if you are in an `asm-func`, the stack will never be used.
+Inside the `rlet`, all uses of `var-name` will always be in the given register.  If the variable goes dead (or is never live), the compiler may reuse the register as it wants.  The compiler may also spill the variable onto the stack.  Of course, if you are in an `asm-func`, the stack will never be used.  Be extremely careful about using "normal" registers without the coloring system and with higher-level code as the compiler may use your "normal" register as a temporary.  If you read the value of a register and use the coloring system, the variable will then be alive starting at the beginning of the function, and will make that register unavailable to the compiler and other `rlet`s that occur before. This is useful to preserve the value of a temporary register if needed, but can also be undesirable in other cases.  If you add the `:reset-here #t` flag, it will make the variable dead until the start of the `rlet`. It "resets" the value of the register in the coloring system at the start of the `rlet`.  The default value is false. It is recommended to keep the default value when accessing specific registers that are also normally used by the compiler.  For special registers like `rsp`, `r15`, `r14`, and `r13`, if you plan to use them with the coloring system, it is recommended to set the `reset-here` flag.
 
 Here is an example of using an `rlet` to access registers:
 ```lisp

goal_src/kernel/gkernel-h.gc

@@ -492,10 +492,16 @@
   )
 
 (defmacro suspend ()
-  `(rlet ((pp :reg r13))
+  `(rlet ((pp :reg r13 :reset-here #t))
          (.push pp)
          (set! pp (-> (the process pp) top-thread))
          ((-> (the cpu-thread pp) suspend-hook) (the cpu-thread 0))
          (.pop pp)
          )
   )
+
+(defmacro process-deactivate ()
+  `(rlet ((pp :reg r13 :reset-here #t :type process))
+         (deactivate pp)
+         )
+  )

goalc/compiler/IR.cpp

@@ -857,15 +857,15 @@ void IR_Null::do_codegen(emitter::ObjectGenerator* gen,
 }
 
 ///////////////////////
-// FunctionStart
+// ValueReset
 ///////////////////////
-IR_FunctionStart::IR_FunctionStart(std::vector<RegVal*> args) : m_args(std::move(args)) {}
+IR_ValueReset::IR_ValueReset(std::vector<RegVal*> args) : m_args(std::move(args)) {}
 
-std::string IR_FunctionStart::print() {
+std::string IR_ValueReset::print() {
-  return "function-start";
+  return "value-reset";
 }
 
-RegAllocInstr IR_FunctionStart::to_rai() {
+RegAllocInstr IR_ValueReset::to_rai() {
   RegAllocInstr rai;
   for (auto& x : m_args) {
     rai.write.push_back(x->ireg());
@@ -873,7 +873,7 @@ RegAllocInstr IR_FunctionStart::to_rai() {
   return rai;
 }
 
-void IR_FunctionStart::do_codegen(emitter::ObjectGenerator* gen,
+void IR_ValueReset::do_codegen(emitter::ObjectGenerator* gen,
                                const AllocationResult& allocs,
                                emitter::IR_Record irec) {
   (void)gen;

goalc/compiler/IR.h

@@ -316,9 +316,9 @@ class IR_Null : public IR {
                   emitter::IR_Record irec) override;
 };
 
-class IR_FunctionStart : public IR {
+class IR_ValueReset : public IR {
  public:
-  IR_FunctionStart(std::vector<RegVal*> args);
+  IR_ValueReset(std::vector<RegVal*> args);
   std::string print() override;
   RegAllocInstr to_rai() override;
   void do_codegen(emitter::ObjectGenerator* gen,

goalc/compiler/compilation/Asm.cpp

@@ -37,6 +37,7 @@ Val* Compiler::compile_rlet(const goos::Object& form, const goos::Object& rest,
   auto lenv = fenv->alloc_env<LexicalEnv>(env);
 
   std::vector<IRegConstraint> constraints;
+  std::vector<RegVal*> reset_regs;
 
   for_each_in_list(defs, [&](const goos::Object& o) {
     // (new-place [:reg old-place] [:type type-spec] [:class reg-type] [:bind #f|lexical|lambda])
@@ -44,6 +45,7 @@ Val* Compiler::compile_rlet(const goos::Object& form, const goos::Object& rest,
     va_check(o, def_args, {goos::ObjectType::SYMBOL},
              {{"reg", {false, goos::ObjectType::SYMBOL}},
               {"type", {false, {}}},
+              {"reset-here", {false, {}}},
               {"class", {false, goos::ObjectType::SYMBOL}}});
 
     // get the name of the new place
@@ -71,11 +73,17 @@ Val* Compiler::compile_rlet(const goos::Object& form, const goos::Object& rest,
     // alloc a register:
     auto new_place_reg = env->make_ireg(ts, register_kind);
     new_place_reg->mark_as_settable();
+
     if (def_args.has_named("reg")) {
       IRegConstraint constraint;
       constraint.ireg = new_place_reg->ireg();
       constraint.contrain_everywhere = true;
       constraint.desired_register = parse_register(def_args.named.at("reg"));
+      if (def_args.has_named("reset-here") &&
+          get_true_or_false(form, def_args.get_named("reset-here"))) {
+        reset_regs.push_back(new_place_reg);
+      }
+
       new_place_reg->set_rlet_constraint(constraint.desired_register);
       constraints.push_back(constraint);
     }
@@ -83,6 +91,10 @@ Val* Compiler::compile_rlet(const goos::Object& form, const goos::Object& rest,
     lenv->vars[new_place_name.as_symbol()->name] = new_place_reg;
   });
 
+  if (!reset_regs.empty()) {
+    lenv->emit_ir<IR_ValueReset>(reset_regs);
+  }
+
   Val* result = get_none();
   for (u64 i = 1; i < args.unnamed.size(); i++) {
     auto& o = args.unnamed.at(i);

goalc/compiler/compilation/Function.cpp

@@ -169,7 +169,7 @@ Val* Compiler::compile_lambda(const goos::Object& form, const goos::Object& rest
     auto func_block_env = new_func_env->alloc_env<BlockEnv>(new_func_env.get(), "#f");
     func_block_env->return_value = return_reg;
     func_block_env->end_label = Label(new_func_env.get());
-    func_block_env->emit(std::make_unique<IR_FunctionStart>(args_for_coloring));
+    func_block_env->emit(std::make_unique<IR_ValueReset>(args_for_coloring));
 
     // compile the function, iterating through the body.
     Val* result = nullptr;

goalc/compiler/compilation/Type.cpp

@@ -182,7 +182,7 @@ Val* Compiler::generate_inspector_for_type(const goos::Object& form, Env* env, T
   constraint.desired_register = emitter::gRegInfo.get_arg_reg(0);  // to the first argument
   method_env->constrain(constraint);
   // Inform the compiler that `input`'s value will be written to `rdi` (first arg register)
-  method_env->emit(std::make_unique<IR_FunctionStart>(std::vector<RegVal*>{input}));
+  method_env->emit(std::make_unique<IR_ValueReset>(std::vector<RegVal*>{input}));
 
   RegVal* type_name = nullptr;
   if (dynamic_cast<BasicType*>(structured_type)) {
@@ -342,7 +342,7 @@ Val* Compiler::compile_defmethod(const goos::Object& form, const goos::Object& _
   auto func_block_env = new_func_env->alloc_env<BlockEnv>(new_func_env.get(), "#f");
   func_block_env->return_value = return_reg;
   func_block_env->end_label = Label(new_func_env.get());
-  func_block_env->emit(std::make_unique<IR_FunctionStart>(args_for_coloring));
+  func_block_env->emit(std::make_unique<IR_ValueReset>(args_for_coloring));
 
   // compile the function!
   Val* result = nullptr;

test/goalc/source_templates/kernel/kernel-test.gc

@@ -1,41 +1,3 @@
-; (defun get-saved-regs ((dest (pointer uint64)))
-;   (rlet ((s0 :reg rbx :type uint)
-;          (s1 :reg rbp :type uint)
-;          (s2 :reg r10 :type uint)
-;          (s3 :reg r11 :type uint)
-;          (s4 :reg r12 :type uint))
-;         (set! (-> dest 0) s0)
-;         (set! (-> dest 1) s1)
-;         (set! (-> dest 2) s2)
-;         (set! (-> dest 3) s3)
-;         (set! (-> dest 4) s4)
-        
-;         )
-;   )
-
-(defmacro with-named-reg-vars-check (&rest body)
-  `(let ((one 1)
-         (two 2)
-         (three 3)
-         (four 4)
-         (five 5)
-         (six 6)
-         (seven 7)
-         (eight 8)
-         (nine 9))
-     ,@body
-     (format 0 "~d ~d ~d ~d ~d ~d " one two three four five six)
-     (format 0 "~d ~d ~d~%" seven eight nine)
-     )
-  
-  )
-
-(defun deactivate-myself ()
-  "Deactivate the current process. A workaround because rlet isn't working well."
-  (rlet ((pp :reg r13 :type process))
-        (deactivate pp)
-        )
-  )
 
 (defun target-function ((a0 uint) (a1 uint) (a2 uint) (a3 uint) (a4 uint) (a5 uint))
   (format #t "TARGET FUNCTION ~D ~D ~D~%" a0 a1 a2)
@@ -49,7 +11,7 @@
     (format #t "proc1: ~D~%" i)
     (when (> i 4)
       (format #t "DEACTIVATE PROC 1~%")
-      (deactivate-myself)
+      (process-deactivate)
       )
     (suspend)
     )
@@ -102,7 +64,7 @@
     (format #t "Stack Alignemnt ~D/16~%" (logand 15 (the uint stack-arr)))
     )
   (if (eq? a0 (the int 0))
-      (deactivate-myself)
+      (process-deactivate)
       )
   'init-child-proc-result
   )
@@ -117,7 +79,7 @@
       )
     )
   
-  (deactivate-myself)
+  (process-deactivate)
   )
 
 (defun kernel-test-2 ()