Use length for offsets to make it more readable

kddnewton · kddnewton · commit d66c977eb37d · 2023-02-02T20:43:29.000-05:00
diff --git a/lib/syntax_tree/yarv/basic_block.rb b/lib/syntax_tree/yarv/basic_block.rb
@@ -32,8 +32,14 @@ def initialize(block_start, insns)
 
       # Yield each instruction in this basic block along with its index from the
       # original instruction sequence.
-      def each_with_index(&block)
-        insns.each.with_index(block_start, &block)
+      def each_with_length
+        return enum_for(:each_with_length) unless block_given?
+
+        length = block_start
+        insns.each do |insn|
+          yield insn, length
+          length += insn.length
+        end
       end
 
       # This method is used to verify that the basic block is well formed. It
diff --git a/lib/syntax_tree/yarv/control_flow_graph.rb b/lib/syntax_tree/yarv/control_flow_graph.rb
@@ -34,24 +34,23 @@ def initialize(iseq, insns, blocks)
 
       def disasm
         fmt = Disassembler.new(iseq)
-        fmt.output.puts "== cfg #{iseq.name}"
+        fmt.output.print("== cfg: #<ISeq:#{iseq.name}@<compiled>:1 ")
+        fmt.output.puts("(#{iseq.line},0)-(#{iseq.line},0)>")
 
         blocks.each do |block|
-          fmt.output.print(block.id)
-
-          unless block.incoming_blocks.empty?
-            fmt.output.print(" # from: #{block.incoming_blocks.map(&:id).join(", ")}")
-          end
-
-          fmt.output.puts
-
-          fmt.with_prefix("    ") { fmt.format_insns!(block.insns) }
+          fmt.output.puts(block.id)
+          fmt.with_prefix("    ") do
+            unless block.incoming_blocks.empty?
+              from = block.incoming_blocks.map(&:id).join(", ")
+              fmt.output.puts("#{fmt.current_prefix}== from: #{from}")
+            end
 
-          dests = block.outgoing_blocks.map(&:id)
-          dests << "leaves" if block.insns.last.leaves?
-          fmt.output.print("        # to: #{dests.join(", ")}") unless dests.empty?
+            fmt.format_insns!(block.insns, block.block_start)
 
-          fmt.output.puts
+            to = block.outgoing_blocks.map(&:id)
+            to << "leaves" if block.insns.last.leaves?
+            fmt.output.puts("#{fmt.current_prefix}== to: #{to.join(", ")}")
+          end
         end
 
         fmt.string
@@ -71,23 +70,34 @@ def self.compile(iseq)
       # This class is responsible for creating a control flow graph from the
       # given instruction sequence.
       class Compiler
-        attr_reader :iseq, :labels, :insns
+        # This is the instruction sequence that is being compiled.
+        attr_reader :iseq
+
+        # This is a hash of indices in the YARV instruction sequence that point
+        # to their corresponding instruction.
+        attr_reader :insns
+
+        # This is a hash of labels that point to their corresponding index into
+        # the YARV instruction sequence. Note that this is not the same as the
+        # index into the list of instructions on the instruction sequence
+        # object. Instead, this is the index into the C array, so it includes
+        # operands.
+        attr_reader :labels
 
         def initialize(iseq)
           @iseq = iseq
 
-          # We need to find all of the instructions that immediately follow
-          # labels so that when we are looking at instructions that branch we
-          # know where they branch to.
+          @insns = {}
           @labels = {}
-          @insns = []
 
+          length = 0
           iseq.insns.each do |insn|
             case insn
             when Instruction
-              @insns << insn
+              @insns[length] = insn
+              length += insn.length
             when InstructionSequence::Label
-              @labels[insn] = @insns.length
+              @labels[insn] = length
             end
           end
         end
@@ -111,15 +121,15 @@ def compile
         def find_basic_block_starts
           block_starts = Set.new([0])
 
-          insns.each_with_index do |insn, index|
+          insns.each do |index, insn|
             branch_targets = insn.branch_targets
 
             if branch_targets.any?
               branch_targets.each do |branch_target|
                 block_starts.add(labels[branch_target])
               end
 
-              block_starts.add(index + 1) if insn.falls_through?
+              block_starts.add(index + insn.length) if insn.falls_through?
             end
           end
 
@@ -131,10 +141,14 @@ def find_basic_block_starts
         def build_basic_blocks
           block_starts = find_basic_block_starts
 
-          block_starts.each_with_index.to_h do |block_start, block_index|
-            block_end = (block_starts[(block_index + 1)..] + [insns.length]).min
-            block_insns = insns[block_start...block_end]
+          length = 0
+          blocks =
+            iseq.insns.grep(Instruction).slice_after do |insn|
+              length += insn.length
+              block_starts.include?(length)
+            end
 
+          block_starts.zip(blocks).to_h do |block_start, block_insns|
             [block_start, BasicBlock.new(block_start, block_insns)]
           end
         end
@@ -150,7 +164,8 @@ def connect_basic_blocks(blocks)
             end
 
             if (insn.branch_targets.empty? && !insn.leaves?) || insn.falls_through?
-              block.outgoing_blocks << blocks.fetch(block_start + block.insns.length)
+              fall_through_start = block_start + block.insns.sum(&:length)
+              block.outgoing_blocks << blocks.fetch(fall_through_start)
             end
 
             block.outgoing_blocks.each do |outgoing_block|
diff --git a/lib/syntax_tree/yarv/data_flow_graph.rb b/lib/syntax_tree/yarv/data_flow_graph.rb
@@ -44,11 +44,11 @@ def disasm
             output.puts "         # in: #{block_flow.in.join(", ")}"
           end
 
-          block.each_with_index do |insn, index|
+          block.each_with_length do |insn, length|
             output.print("    ")
             output.print(insn.disasm(fmt))
 
-            insn_flow = insn_flows[index]
+            insn_flow = insn_flows[length]
             if insn_flow.in.empty? && insn_flow.out.empty?
               output.puts
               next
@@ -120,8 +120,8 @@ def initialize(cfg)
           # This data structure will hold the data flow between instructions
           # within individual basic blocks.
           @insn_flows = {}
-          cfg.insns.each_with_index do |insn, index|
-            @insn_flows[index] = DataFlow.new
+          cfg.insns.each_key do |length|
+            @insn_flows[length] = DataFlow.new
           end
 
           # This data structure will hold the data flow between basic blocks.
@@ -147,8 +147,8 @@ def find_local_flow
             stack = []
 
             # Go through each instruction in the block...
-            block.each_with_index do |insn, index|
-              insn_flow = insn_flows[index]
+            block.each_with_length do |insn, length|
+              insn_flow = insn_flows[length]
 
               # How many values will be missing from the local stack to run this
               # instruction?
@@ -172,7 +172,7 @@ def find_local_flow
 
               # Record on our abstract stack that this instruction pushed
               # this value onto the stack.
-              insn.pushes.times { stack << index }
+              insn.pushes.times { stack << length }
             end
 
             # Values that are left on the stack after going through all
@@ -184,16 +184,16 @@ def find_local_flow
           end
 
           # Go backwards and connect from producers to consumers.
-          cfg.insns.each_with_index do |insn, index|
+          cfg.insns.each_key do |length|
             # For every instruction that produced a value used in this
             # instruction...
-            insn_flows[index].in.each do |producer|
+            insn_flows[length].in.each do |producer|
               # If it's actually another instruction and not a basic block
               # argument...
               if producer.is_a?(Integer)
                 # Record in the producing instruction that it produces a value
                 # used by this construction.
-                insn_flows[producer].out << index
+                insn_flows[producer].out << length
               end
             end
           end
diff --git a/test/yarv_test.rb b/test/yarv_test.rb
@@ -303,25 +303,28 @@ def test_cfg
       cfg = SyntaxTree::YARV::ControlFlowGraph.compile(iseq)
 
       assert_equal(<<~CFG, cfg.disasm)
-        == cfg <compiled>
+        == cfg: #<ISeq:<compiled>@<compiled>:1 (1,0)-(1,0)>
         block_0
             0000 putobject                              100
             0002 putobject                              14
             0004 putobject_INT2FIX_0_
             0005 opt_lt                                 <calldata!mid:<, argc:1, ARGS_SIMPLE>
             0007 branchunless                           13
-                # to: block_7, block_5
-        block_5 # from: block_0
-            0000 putobject                              -1
-            0002 jump                                   14
-                # to: block_8
-        block_7 # from: block_0
-            0000 putobject_INT2FIX_1_
-                # to: block_8
-        block_8 # from: block_5, block_7
-            0000 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>
-            0002 leave
-                # to: leaves
+            == to: block_13, block_9
+        block_9
+            == from: block_0
+            0009 putobject                              -1
+            0011 jump                                   14
+            == to: block_14
+        block_13
+            == from: block_0
+            0013 putobject_INT2FIX_1_
+            == to: block_14
+        block_14
+            == from: block_9, block_13
+            0014 opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE>
+            0016 leave
+            == to: leaves
       CFG
     end
 
@@ -335,27 +338,27 @@ def test_dfg
         == dfg <compiled>
         block_0
             putobject                              100 # out: out_0
-            putobject                              14 # out: 3
-            putobject_INT2FIX_0_ # out: 3
-            opt_lt                                 <calldata!mid:<, argc:1, ARGS_SIMPLE> # in: 1, 2; out: 4
-            branchunless                           13 # in: 3
-                # to: block_7, block_5
+            putobject                              14 # out: 5
+            putobject_INT2FIX_0_ # out: 5
+            opt_lt                                 <calldata!mid:<, argc:1, ARGS_SIMPLE> # in: 2, 4; out: 7
+            branchunless                           13 # in: 5
+                # to: block_13, block_9
                 # out: 0
-        block_5 # from: block_0
+        block_9 # from: block_0
                  # in: pass_0
             putobject                              -1 # out: out_0
             jump                                   14
-                # to: block_8
-                # out: pass_0, 5
-        block_7 # from: block_0
+                # to: block_14
+                # out: pass_0, 9
+        block_13 # from: block_0
                  # in: pass_0
             putobject_INT2FIX_1_ # out: out_0
-                # to: block_8
-                # out: pass_0, 7
-        block_8 # from: block_5, block_7
+                # to: block_14
+                # out: pass_0, 13
+        block_14 # from: block_9, block_13
                  # in: in_0, in_1
-            opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE> # in: in_0, in_1; out: 9
-            leave # in: 8
+            opt_plus                               <calldata!mid:+, argc:1, ARGS_SIMPLE> # in: in_0, in_1; out: 16
+            leave # in: 14
                 # to: leaves
       DFG
     end
