Relax block ordering constraints (#1535)

* Relax block ordering constraints

Before `block.Next` had to follow `block.ListNext`, now it does not.
Instead `CodeGenerator` will now emit the necessary jump instructions
to ensure control flow.

This makes control flow and block order modifications easier. It also
eliminates some simple cases of redundant branches.

* Set PPTC version

ARMeilleure/CodeGen/RegisterAllocators/HybridAllocator.cs

@@ -427,18 +427,5 @@ namespace ARMeilleure.CodeGen.RegisterAllocators
         {
             return local.Assignments.Count + local.Uses.Count;
         }
-
-        private static IEnumerable<BasicBlock> Successors(BasicBlock block)
-        {
-            if (block.Next != null)
-            {
-                yield return block.Next;
-            }
-
-            if (block.Branch != null)
-            {
-                yield return block.Branch;
-            }
-        }
     }
 }

ARMeilleure/CodeGen/RegisterAllocators/LinearScanAllocator.cs

@@ -626,16 +626,11 @@ namespace ARMeilleure.CodeGen.RegisterAllocators
                     continue;
                 }
 
-                bool hasSingleOrNoSuccessor = block.Next == null || block.Branch == null;
+                bool hasSingleOrNoSuccessor = block.SuccessorCount <= 1;
 
-                for (int i = 0; i < 2; i++)
+                for (int i = 0; i < block.SuccessorCount; i++)
                 {
-                    // This used to use an enumerable, but it ended up generating a lot of garbage, so now it is a loop.
-                    BasicBlock successor = (i == 0) ? block.Next : block.Branch;
-                    if (successor == null)
-                    {
-                        continue;
-                    }
+                    BasicBlock successor = block.GetSuccessor(i);
 
                     int succIndex = successor.Index;
 
@@ -643,7 +638,7 @@ namespace ARMeilleure.CodeGen.RegisterAllocators
                     // (the successor before the split) should be right after it.
                     if (IsSplitEdgeBlock(successor))
                     {
-                        succIndex = FirstSuccessor(successor).Index;
+                        succIndex = successor.GetSuccessor(0).Index;
                     }
 
                     CopyResolver copyResolver = new CopyResolver();
@@ -883,10 +878,11 @@ namespace ARMeilleure.CodeGen.RegisterAllocators
 
                     BitMap liveOut = blkLiveOut[block.Index];
 
-                    if ((block.Next != null && liveOut.Set(blkLiveIn[block.Next.Index])) || 
-                        (block.Branch != null && liveOut.Set(blkLiveIn[block.Branch.Index])))
+                    for (int i = 0; i < block.SuccessorCount; i++)
                     {
-                        modified = true;
+                        BasicBlock succ = block.GetSuccessor(i);
+
+                        modified |= liveOut.Set(blkLiveIn[succ.Index]);
                     }
 
                     BitMap liveIn = blkLiveIn[block.Index];
@@ -1002,11 +998,6 @@ namespace ARMeilleure.CodeGen.RegisterAllocators
             return (register.Index << 1) | (register.Type == RegisterType.Vector ? 1 : 0);
         }
 
-        private static BasicBlock FirstSuccessor(BasicBlock block)
-        {
-            return block.Next ?? block.Branch;
-        }
-
         private static IEnumerable<Node> BottomOperations(BasicBlock block)
         {
             Node node = block.Operations.Last;

ARMeilleure/CodeGen/X86/CodeGenerator.cs

@@ -32,7 +32,6 @@ namespace ARMeilleure.CodeGen.X86
             Add(Instruction.BitwiseExclusiveOr,      GenerateBitwiseExclusiveOr);
             Add(Instruction.BitwiseNot,              GenerateBitwiseNot);
             Add(Instruction.BitwiseOr,               GenerateBitwiseOr);
-            Add(Instruction.Branch,                  GenerateBranch);
             Add(Instruction.BranchIf,                GenerateBranchIf);
             Add(Instruction.ByteSwap,                GenerateByteSwap);
             Add(Instruction.Call,                    GenerateCall);
@@ -168,6 +167,23 @@ namespace ARMeilleure.CodeGen.X86
                             GenerateOperation(context, operation);
                         }
                     }
+
+                    if (block.SuccessorCount == 0)
+                    {
+                        // The only blocks which can have 0 successors are exit blocks.
+                        Debug.Assert(block.Operations.Last is Operation operation &&
+                                     (operation.Instruction == Instruction.Tailcall ||
+                                      operation.Instruction == Instruction.Return));
+                    }
+                    else
+                    {
+                        BasicBlock succ = block.GetSuccessor(0);
+
+                        if (succ != block.ListNext)
+                        {
+                            context.JumpTo(succ);
+                        }
+                    }
                 }
 
                 Logger.EndPass(PassName.CodeGeneration);
@@ -512,11 +528,6 @@ namespace ARMeilleure.CodeGen.X86
             context.Assembler.Or(dest, src2, dest.Type);
         }
 
-        private static void GenerateBranch(CodeGenContext context, Operation operation)
-        {
-            context.JumpTo(context.CurrBlock.Branch);
-        }
-
         private static void GenerateBranchIf(CodeGenContext context, Operation operation)
         {
             Operand comp = operation.GetSource(2);
@@ -527,7 +538,7 @@ namespace ARMeilleure.CodeGen.X86
 
             GenerateCompareCommon(context, operation);
 
-            context.JumpTo(cond, context.CurrBlock.Branch);
+            context.JumpTo(cond, context.CurrBlock.GetSuccessor(1));
         }
 
         private static void GenerateByteSwap(CodeGenContext context, Operation operation)

ARMeilleure/Diagnostics/IRDumper.cs

@@ -57,17 +57,20 @@ namespace ARMeilleure.Diagnostics
         {
             DumpBlockName(block);
 
-            if (block.Next != null)
+            if (block.SuccessorCount > 0)
             {
-                _builder.Append(" (next ");
-                DumpBlockName(block.Next);
-                _builder.Append(')');
-            }
+                _builder.Append(" (");
+
+                for (int i = 0; i < block.SuccessorCount; i++)
+                {
+                    DumpBlockName(block.GetSuccessor(i));
+
+                    if (i < block.SuccessorCount - 1)
+                    {
+                        _builder.Append(", ");
+                    }
+                }
 
-            if (block.Branch != null)
-            {
-                _builder.Append(" (branch ");
-                DumpBlockName(block.Branch);
                 _builder.Append(')');
             }
 

ARMeilleure/IntermediateRepresentation/BasicBlock.cs

@@ -1,9 +1,12 @@
+using System;
 using System.Collections.Generic;
 
 namespace ARMeilleure.IntermediateRepresentation
 {
     class BasicBlock : IIntrusiveListNode<BasicBlock>
     {
+        private readonly List<BasicBlock> _successors = new List<BasicBlock>();
+
         public int Index { get; set; }
 
         public BasicBlock ListPrevious { get; set; }
@@ -11,69 +14,82 @@ namespace ARMeilleure.IntermediateRepresentation
 
         public IntrusiveList<Node> Operations { get; }
 
-        private BasicBlock _next;
-        private BasicBlock _branch;
-
-        public BasicBlock Next
-        {
-            get => _next;
-            set => _next = AddSuccessor(_next, value);
-        }
-
-        public BasicBlock Branch
-        {
-            get => _branch;
-            set => _branch = AddSuccessor(_branch, value);
-        }
-
         public List<BasicBlock> Predecessors { get; }
 
         public HashSet<BasicBlock> DominanceFrontiers { get; }
-
         public BasicBlock ImmediateDominator { get; set; }
 
-        public BasicBlock()
+        public int SuccessorCount => _successors.Count;
+
+        public BasicBlock() : this(index: -1) { }
+
+        public BasicBlock(int index)
         {
             Operations = new IntrusiveList<Node>();
-
             Predecessors = new List<BasicBlock>();
-
             DominanceFrontiers = new HashSet<BasicBlock>();
 
-            Index = -1;
+            Index = index;
         }
 
-        public BasicBlock(int index) : this()
+        public void AddSuccessor(BasicBlock block)
         {
-            Index = index;
+            if (block == null)
+            {
+                throw new ArgumentNullException(nameof(block));
+            }
+
+            block.Predecessors.Add(this);
+
+            _successors.Add(block);
         }
 
-        private BasicBlock AddSuccessor(BasicBlock oldBlock, BasicBlock newBlock)
+        public void RemoveSuccessor(int index)
         {
-            oldBlock?.Predecessors.Remove(this);
-            newBlock?.Predecessors.Add(this);
+            BasicBlock oldBlock = _successors[index];
 
-            return newBlock;
+            oldBlock.Predecessors.Remove(this);
+
+            _successors.RemoveAt(index);
         }
 
-        public void Append(Node node)
+        public BasicBlock GetSuccessor(int index)
         {
-            // If the branch block is not null, then the list of operations
-            // should end with a branch instruction. We insert the new operation
-            // before this branch.
-            if (_branch != null || (Operations.Last != null && IsLeafBlock()))
-            {
-                Operations.AddBefore(Operations.Last, node);
-            }
-            else
+            return _successors[index];
+        }
+
+        public void SetSuccessor(int index, BasicBlock block)
+        {
+            if (block == null)
             {
-                Operations.AddLast(node);
+                throw new ArgumentNullException(nameof(block));
             }
+
+            BasicBlock oldBlock = _successors[index];
+
+            oldBlock.Predecessors.Remove(this);
+            block.Predecessors.Add(this);
+
+            _successors[index] = block;
         }
 
-        private bool IsLeafBlock()
+        public void Append(Node node)
         {
-            return _branch == null && _next == null;
+            var lastOp = Operations.Last as Operation;
+
+            // Append node before terminal or to end if no terminal.
+            switch (lastOp?.Instruction)
+            {
+                case Instruction.Return:
+                case Instruction.Tailcall:
+                case Instruction.BranchIf:
+                    Operations.AddBefore(lastOp, node);
+                    break;
+
+                default:
+                    Operations.AddLast(node);
+                    break;
+            }
         }
 
         public Node GetLastOp()

ARMeilleure/IntermediateRepresentation/Instruction.cs

@@ -7,7 +7,6 @@ namespace ARMeilleure.IntermediateRepresentation
         BitwiseExclusiveOr,
         BitwiseNot,
         BitwiseOr,
-        Branch,
         BranchIf,
         ByteSwap,
         Call,

ARMeilleure/Translation/ControlFlowGraph.cs

@@ -8,47 +8,48 @@ namespace ARMeilleure.Translation
     class ControlFlowGraph
     {
         public BasicBlock Entry { get; }
-
         public IntrusiveList<BasicBlock> Blocks { get; }
-
         public BasicBlock[] PostOrderBlocks { get; }
-
         public int[] PostOrderMap { get; }
 
         public ControlFlowGraph(BasicBlock entry, IntrusiveList<BasicBlock> blocks)
         {
-            Entry  = entry;
+            Entry = entry;
             Blocks = blocks;
 
             RemoveUnreachableBlocks(blocks);
 
-            HashSet<BasicBlock> visited = new HashSet<BasicBlock>();
-
-            Stack<BasicBlock> blockStack = new Stack<BasicBlock>();
+            var visited = new HashSet<BasicBlock>();
+            var blockStack = new Stack<BasicBlock>();
 
             PostOrderBlocks = new BasicBlock[blocks.Count];
-
             PostOrderMap = new int[blocks.Count];
 
             visited.Add(entry);
-
             blockStack.Push(entry);
 
             int index = 0;
 
             while (blockStack.TryPop(out BasicBlock block))
             {
-                if (block.Next != null && visited.Add(block.Next))
-                {
-                    blockStack.Push(block);
-                    blockStack.Push(block.Next);
-                }
-                else if (block.Branch != null && visited.Add(block.Branch))
+                bool visitedNew = false;
+
+                for (int i = 0; i < block.SuccessorCount; i++)
                 {
-                    blockStack.Push(block);
-                    blockStack.Push(block.Branch);
+                    BasicBlock succ = block.GetSuccessor(i);
+
+                    if (visited.Add(succ))
+                    {
+                        blockStack.Push(block);
+                        blockStack.Push(succ);
+
+                        visitedNew = true;
+
+                        break;
+                    }
                 }
-                else
+
+                if (!visitedNew)
                 {
                     PostOrderMap[block.Index] = index;
 
@@ -59,26 +60,24 @@ namespace ARMeilleure.Translation
 
         private void RemoveUnreachableBlocks(IntrusiveList<BasicBlock> blocks)
         {
-            HashSet<BasicBlock> visited = new HashSet<BasicBlock>();
-
-            Queue<BasicBlock> workQueue = new Queue<BasicBlock>();
+            var visited = new HashSet<BasicBlock>();
+            var workQueue = new Queue<BasicBlock>();
 
             visited.Add(Entry);
-
             workQueue.Enqueue(Entry);
 
             while (workQueue.TryDequeue(out BasicBlock block))
             {
                 Debug.Assert(block.Index != -1, "Invalid block index.");
 
-                if (block.Next != null && visited.Add(block.Next))
+                for (int i = 0; i < block.SuccessorCount; i++)
                 {
-                    workQueue.Enqueue(block.Next);
-                }
+                    BasicBlock succ = block.GetSuccessor(i);
 
-                if (block.Branch != null && visited.Add(block.Branch))
-                {
-                    workQueue.Enqueue(block.Branch);
+                    if (visited.Add(succ))
+                    {
+                        workQueue.Enqueue(succ);
+                    }
                 }
             }
 
@@ -93,8 +92,10 @@ namespace ARMeilleure.Translation
 
                     if (!visited.Contains(block))
                     {
-                        block.Next = null;
-                        block.Branch = null;
+                        while (block.SuccessorCount > 0)
+                        {
+                            block.RemoveSuccessor(index: block.SuccessorCount - 1);
+                        }
 
                         blocks.Remove(block);
                     }
@@ -112,14 +113,12 @@ namespace ARMeilleure.Translation
         {
             BasicBlock splitBlock = new BasicBlock(Blocks.Count);
 
-            if (predecessor.Next == successor)
+            for (int i = 0; i < predecessor.SuccessorCount; i++)
             {
-                predecessor.Next = splitBlock;
-            }
-
-            if (predecessor.Branch == successor)
-            {
-                predecessor.Branch = splitBlock;
+                if (predecessor.GetSuccessor(i) == successor)
+                {
+                    predecessor.SetSuccessor(i, splitBlock);
+                }
             }
 
             if (splitBlock.Predecessors.Count == 0)
@@ -127,26 +126,7 @@ namespace ARMeilleure.Translation
                 throw new ArgumentException("Predecessor and successor are not connected.");
             }
 
-            // Insert the new block on the list of blocks.
-            BasicBlock succPrev = successor.ListPrevious;
-
-            if (succPrev != null && succPrev != predecessor && succPrev.Next == successor)
-            {
-                // Can't insert after the predecessor or before the successor.
-                // Here, we insert it before the successor by also spliting another
-                // edge (the one between the block before "successor" and "successor").
-                BasicBlock splitBlock2 = new BasicBlock(splitBlock.Index + 1);
-
-                succPrev.Next = splitBlock2;
-
-                splitBlock2.Branch = successor;
-
-                splitBlock2.Operations.AddLast(OperationHelper.Operation(Instruction.Branch, null));
-
-                Blocks.AddBefore(successor, splitBlock2);
-            }
-
-            splitBlock.Next = successor;
+            splitBlock.AddSuccessor(successor);
 
             Blocks.AddBefore(successor, splitBlock);
 

ARMeilleure/Translation/EmitterContext.cs

@@ -13,18 +13,17 @@ namespace ARMeilleure.Translation
 
     class EmitterContext
     {
-        private Dictionary<Operand, BasicBlock> _irLabels;
-
-        private IntrusiveList<BasicBlock> _irBlocks;
+        private readonly Dictionary<Operand, BasicBlock> _irLabels;
+        private readonly IntrusiveList<BasicBlock> _irBlocks;
 
         private BasicBlock _irBlock;
+        private BasicBlock _ifBlock;
 
         private bool _needsNewBlock;
 
         public EmitterContext()
         {
             _irLabels = new Dictionary<Operand, BasicBlock>();
-
             _irBlocks = new IntrusiveList<BasicBlock>();
 
             _needsNewBlock = true;
@@ -57,16 +56,16 @@ namespace ARMeilleure.Translation
 
         public void Branch(Operand label)
         {
-            Add(Instruction.Branch, null);
+            NewNextBlockIfNeeded();
 
-            BranchToLabel(label);
+            BranchToLabel(label, uncond: true);
         }
 
         public void BranchIf(Operand label, Operand op1, Operand op2, Comparison comp)
         {
             Add(Instruction.BranchIf, null, op1, op2, Const((int)comp));
 
-            BranchToLabel(label);
+            BranchToLabel(label, uncond: false);
         }
 
         public void BranchIfFalse(Operand label, Operand op1)
@@ -574,10 +573,7 @@ namespace ARMeilleure.Translation
 
         private Operand Add(Intrinsic intrin, Operand dest, params Operand[] sources)
         {
-            if (_needsNewBlock)
-            {
-                NewNextBlock();
-            }
+            NewNextBlockIfNeeded();
 
             IntrinsicOperation operation = new IntrinsicOperation(intrin, dest, sources);
 
@@ -586,7 +582,7 @@ namespace ARMeilleure.Translation
             return dest;
         }
 
-        private void BranchToLabel(Operand label)
+        private void BranchToLabel(Operand label, bool uncond)
         {
             if (!_irLabels.TryGetValue(label, out BasicBlock branchBlock))
             {
@@ -595,7 +591,15 @@ namespace ARMeilleure.Translation
                 _irLabels.Add(label, branchBlock);
             }
 
-            _irBlock.Branch = branchBlock;
+            if (uncond)
+            {
+                _irBlock.AddSuccessor(branchBlock);
+            }
+            else
+            {
+                // Defer registration of successor to _irBlock so that the order of successors is correct.
+                _ifBlock = branchBlock;
+            }
 
             _needsNewBlock = true;
         }
@@ -629,9 +633,16 @@ namespace ARMeilleure.Translation
 
         private void NextBlock(BasicBlock nextBlock)
         {
-            if (_irBlock != null && !EndsWithUnconditional(_irBlock))
+            if (_irBlock != null && _irBlock.SuccessorCount == 0 && !EndsWithUnconditional(_irBlock))
             {
-                _irBlock.Next = nextBlock;
+                _irBlock.AddSuccessor(nextBlock);
+
+                if (_ifBlock != null)
+                {
+                    _irBlock.AddSuccessor(_ifBlock);
+
+                    _ifBlock = null;
+                }
             }
 
             _irBlock = nextBlock;
@@ -642,8 +653,7 @@ namespace ARMeilleure.Translation
         private static bool EndsWithUnconditional(BasicBlock block)
         {
             return block.Operations.Last is Operation lastOp &&
-                   (lastOp.Instruction == Instruction.Branch ||
-                    lastOp.Instruction == Instruction.Return ||
+                   (lastOp.Instruction == Instruction.Return ||
                     lastOp.Instruction == Instruction.Tailcall);
         }
 

ARMeilleure/Translation/PTC/Ptc.cs

@@ -21,7 +21,7 @@ namespace ARMeilleure.Translation.PTC
     {
         private const string HeaderMagic = "PTChd";
 
-        private const int InternalVersion = 1537; //! To be incremented manually for each change to the ARMeilleure project.
+        private const int InternalVersion = 1535; //! To be incremented manually for each change to the ARMeilleure project.
 
         private const string ActualDir = "0";
         private const string BackupDir = "1";

ARMeilleure/Translation/RegisterUsage.cs

@@ -171,14 +171,9 @@ namespace ARMeilleure.Translation
 
                     RegisterMask inputs = localInputs[block.Index];
 
-                    if (block.Next != null)
+                    for (int i = 0; i < block.SuccessorCount; i++)
                     {
-                        inputs |= globalInputs[block.Next.Index];
-                    }
-
-                    if (block.Branch != null)
-                    {
-                        inputs |= globalInputs[block.Branch.Index];
+                        inputs |= globalInputs[block.GetSuccessor(i).Index];
                     }
 
                     inputs &= ~globalCmnOutputs[block.Index];

ARMeilleure/Translation/Translator.cs

@@ -309,8 +309,6 @@ namespace ARMeilleure.Translation
 
             context.Return(Const(0L));
 
-            context.Branch(lblExit);
-
             context.MarkLabel(lblNonZero);
 
             count = context.Subtract(count, Const(1));