From f13e6dcf57240ff38e4111efe08abbeebc550492 Mon Sep 17 00:00:00 2001
From: Torsten <torsten@rubydesign.fi>
Date: Thu, 19 Mar 2020 14:10:39 +0200
Subject: [PATCH] fix releasing in allocator

fell into hash new trap, which reuses the object you give it. not good for mutable objects like the array.
also previous logic was broken in terms of machine vs ssa names
---
 lib/risc/standard_allocator.rb        | 47 +++++++++++++++++++++------
 test/risc/test_standard_allocator.rb  |  4 +++
 test/risc/test_standard_allocator1.rb |  2 +-
 3 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/lib/risc/standard_allocator.rb b/lib/risc/standard_allocator.rb
index 2d70c583..cb8f931f 100644
--- a/lib/risc/standard_allocator.rb
+++ b/lib/risc/standard_allocator.rb
@@ -13,7 +13,7 @@ module Risc
       @compiler = compiler
       @platform = platform
       @used_regs = {}
-      @release_points = Hash.new( [] )
+      @release_points = Hash.new {|hash , key | hash[key] = [] }
       @reg_names = (0 ... platform.num_registers).collect{|i| "r#{i}".to_sym }
     end
     attr_reader :used_regs , :compiler , :platform , :reg_names
@@ -28,11 +28,25 @@ module Risc
       pointer = @compiler.risc_instructions
       while(pointer)
         names = assign(pointer)
-        names.each {|name| release_reg(name)}
+        names.each {|name| release_after(pointer, name)}
         pointer = pointer.next
       end
     end
 
+    # if the instruction has a release point with the given name, release it
+    # Release points store the last use of a register (in ssa)
+    # This method is called after machine registers have been assigned,
+    # and give us the chance to reclaim any unsued machine regs
+    # (via the precalculated release_points)
+    def release_after(instruction , ssa_name)
+      release = @release_points[instruction]
+      return unless release
+      return unless release.include?(ssa_name)
+      #puts "ReleasePoint #{ssa_name} for #{instruction} #{release}"
+      pois = reverse_used(ssa_name)
+      release_reg( pois ) if pois
+    end
+
     def assign(instruction)
       names = instruction.register_names
       names.each do |for_name|
@@ -52,10 +66,11 @@ module Risc
     def walk_and_mark(instruction)
       released = []
       released = walk_and_mark(instruction.next) if instruction.next
-      #puts instruction.class.name
+      #puts "Walking #{instruction}"
       instruction.register_names.each do |name|
         next if released.include?(name)
         @release_points[instruction] << name
+        #puts "ADDING #{name}"
         released << name
       end
       released
@@ -65,21 +80,32 @@ module Risc
       @used_regs.empty?
     end
 
-    def use_reg(reg , for_name)
+    # use the given reg (first) parameter and mark it as assigned to
+    # it's ssa form, the second parameter.
+    # forward check is trivial, and reverse_used provides reverse check
+    def use_reg(reg , ssa_name)
       reg = reg.symbol if reg.is_a?(RegisterValue)
       raise "Stupid error #{reg}" unless reg.is_a?(Symbol)
-      puts "Using #{reg} for #{for_name}"
-      @used_regs[reg] = for_name
+      #puts "Using #{reg} for #{ssa_name}"
+      @used_regs[reg] = ssa_name
     end
 
-    # if a register has been assigned to the given name, return that
+    # Check whether a register has been assigned to the given ssa form given.
+    # Ie a reverse check on the used_regs hash
+    def reverse_used( ssa_name )
+      @used_regs.each {|reg,name| return reg if ssa_name == name }
+      return nil
+    end
+
+    # if a register has been assigned to the given ssa name, return that
     #
     # otherwise find the first free register by going through the available names
     # and checking if it is used
-    def get_reg(for_name)
-      @used_regs.each {|reg,name| return reg if for_name == name }
+    def get_reg(ssa_name)
+      name = reverse_used( ssa_name )
+      return name if name
       @reg_names.each do |name|
-        return use_reg(name , for_name) unless @used_regs.has_key?(name)
+        return use_reg(name , ssa_name) unless @used_regs.has_key?(name)
       end
       raise "No more registers #{self}"
     end
@@ -89,6 +115,7 @@ module Risc
     def release_reg( reg )
       reg = reg.symbol if reg.is_a?(RegisterValue)
       raise "not symbol #{reg}:#{reg.class}" unless reg.is_a?(Symbol)
+      #puts "Releasing #{reg} "
       @used_regs.delete(reg)
     end
 
diff --git a/test/risc/test_standard_allocator.rb b/test/risc/test_standard_allocator.rb
index 8ce80ca0..615821f4 100644
--- a/test/risc/test_standard_allocator.rb
+++ b/test/risc/test_standard_allocator.rb
@@ -37,6 +37,10 @@ module Risc
       assert_equal Symbol, @allocator.use_reg(:r1, :some).class
       assert @allocator.used_regs.include?(:r1)
     end
+    def test_reverse_check
+      assert_equal Symbol, @allocator.use_reg(:r1, :some).class
+      assert_equal :r1 , @allocator.reverse_used(:some)
+    end
     def test_add_fail
       assert_raises{ @allocator.use_reg(1)}
     end
diff --git a/test/risc/test_standard_allocator1.rb b/test/risc/test_standard_allocator1.rb
index b65116e7..44372ab3 100644
--- a/test/risc/test_standard_allocator1.rb
+++ b/test/risc/test_standard_allocator1.rb
@@ -13,7 +13,7 @@ module Risc
     end
     def test_allocate_runs
       assert_nil @allocator.allocate_regs
-      assert_equal 10 , @allocator.used_regs.length
+      assert_equal 0 , @allocator.used_regs.length
     end
     def test_live_length
       live = @allocator.walk_and_mark(@compiler.risc_instructions)