From 36ec1bc6c023811235d9f5fb664feff09bc7b4f7 Mon Sep 17 00:00:00 2001 From: FICTURE7 Date: Sat, 12 Sep 2020 19:32:53 +0400 Subject: [PATCH] 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 --- .../RegisterAllocators/HybridAllocator.cs | 13 --- .../RegisterAllocators/LinearScanAllocator.cs | 25 ++--- ARMeilleure/CodeGen/X86/CodeGenerator.cs | 25 +++-- ARMeilleure/Diagnostics/IRDumper.cs | 21 ++-- .../IntermediateRepresentation/BasicBlock.cs | 102 ++++++++++-------- .../IntermediateRepresentation/Instruction.cs | 1 - ARMeilleure/Translation/ControlFlowGraph.cs | 94 +++++++--------- ARMeilleure/Translation/EmitterContext.cs | 44 +++++--- ARMeilleure/Translation/PTC/Ptc.cs | 2 +- ARMeilleure/Translation/RegisterUsage.cs | 9 +- ARMeilleure/Translation/Translator.cs | 2 - 11 files changed, 164 insertions(+), 174 deletions(-) diff --git a/ARMeilleure/CodeGen/RegisterAllocators/HybridAllocator.cs b/ARMeilleure/CodeGen/RegisterAllocators/HybridAllocator.cs index 21470a663..898cc1db8 100644 --- a/ARMeilleure/CodeGen/RegisterAllocators/HybridAllocator.cs +++ b/ARMeilleure/CodeGen/RegisterAllocators/HybridAllocator.cs @@ -427,18 +427,5 @@ namespace ARMeilleure.CodeGen.RegisterAllocators { return local.Assignments.Count + local.Uses.Count; } - - private static IEnumerable Successors(BasicBlock block) - { - if (block.Next != null) - { - yield return block.Next; - } - - if (block.Branch != null) - { - yield return block.Branch; - } - } } } \ No newline at end of file diff --git a/ARMeilleure/CodeGen/RegisterAllocators/LinearScanAllocator.cs b/ARMeilleure/CodeGen/RegisterAllocators/LinearScanAllocator.cs index 01bb9554d..71739d436 100644 --- a/ARMeilleure/CodeGen/RegisterAllocators/LinearScanAllocator.cs +++ b/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 BottomOperations(BasicBlock block) { Node node = block.Operations.Last; diff --git a/ARMeilleure/CodeGen/X86/CodeGenerator.cs b/ARMeilleure/CodeGen/X86/CodeGenerator.cs index f2d4c4627..a51f4a137 100644 --- a/ARMeilleure/CodeGen/X86/CodeGenerator.cs +++ b/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) diff --git a/ARMeilleure/Diagnostics/IRDumper.cs b/ARMeilleure/Diagnostics/IRDumper.cs index 0f0d72c15..90eb2300a 100644 --- a/ARMeilleure/Diagnostics/IRDumper.cs +++ b/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(')'); } diff --git a/ARMeilleure/IntermediateRepresentation/BasicBlock.cs b/ARMeilleure/IntermediateRepresentation/BasicBlock.cs index ac48ac8eb..640978fe7 100644 --- a/ARMeilleure/IntermediateRepresentation/BasicBlock.cs +++ b/ARMeilleure/IntermediateRepresentation/BasicBlock.cs @@ -1,9 +1,12 @@ +using System; using System.Collections.Generic; namespace ARMeilleure.IntermediateRepresentation { class BasicBlock : IIntrusiveListNode { + private readonly List _successors = new List(); + public int Index { get; set; } public BasicBlock ListPrevious { get; set; } @@ -11,69 +14,82 @@ namespace ARMeilleure.IntermediateRepresentation public IntrusiveList 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 Predecessors { get; } public HashSet 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(); - Predecessors = new List(); - DominanceFrontiers = new HashSet(); - Index = -1; - } - - public BasicBlock(int index) : this() - { Index = index; } - private BasicBlock AddSuccessor(BasicBlock oldBlock, BasicBlock newBlock) + public void AddSuccessor(BasicBlock block) { - oldBlock?.Predecessors.Remove(this); - newBlock?.Predecessors.Add(this); + if (block == null) + { + throw new ArgumentNullException(nameof(block)); + } - return newBlock; + block.Predecessors.Add(this); + + _successors.Add(block); + } + + public void RemoveSuccessor(int index) + { + BasicBlock oldBlock = _successors[index]; + + oldBlock.Predecessors.Remove(this); + + _successors.RemoveAt(index); + } + + public BasicBlock GetSuccessor(int index) + { + return _successors[index]; + } + + public void SetSuccessor(int index, BasicBlock block) + { + if (block == null) + { + throw new ArgumentNullException(nameof(block)); + } + + BasicBlock oldBlock = _successors[index]; + + oldBlock.Predecessors.Remove(this); + block.Predecessors.Add(this); + + _successors[index] = block; } public void Append(Node node) { - // 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 - { - Operations.AddLast(node); - } - } + var lastOp = Operations.Last as Operation; - private bool IsLeafBlock() - { - return _branch == null && _next == null; + // 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() diff --git a/ARMeilleure/IntermediateRepresentation/Instruction.cs b/ARMeilleure/IntermediateRepresentation/Instruction.cs index c583a2f2b..938a528df 100644 --- a/ARMeilleure/IntermediateRepresentation/Instruction.cs +++ b/ARMeilleure/IntermediateRepresentation/Instruction.cs @@ -7,7 +7,6 @@ namespace ARMeilleure.IntermediateRepresentation BitwiseExclusiveOr, BitwiseNot, BitwiseOr, - Branch, BranchIf, ByteSwap, Call, diff --git a/ARMeilleure/Translation/ControlFlowGraph.cs b/ARMeilleure/Translation/ControlFlowGraph.cs index 16b406ab0..349635344 100644 --- a/ARMeilleure/Translation/ControlFlowGraph.cs +++ b/ARMeilleure/Translation/ControlFlowGraph.cs @@ -8,47 +8,48 @@ namespace ARMeilleure.Translation class ControlFlowGraph { public BasicBlock Entry { get; } - public IntrusiveList Blocks { get; } - public BasicBlock[] PostOrderBlocks { get; } - public int[] PostOrderMap { get; } public ControlFlowGraph(BasicBlock entry, IntrusiveList blocks) { - Entry = entry; + Entry = entry; Blocks = blocks; RemoveUnreachableBlocks(blocks); - HashSet visited = new HashSet(); - - Stack blockStack = new Stack(); + var visited = new HashSet(); + var blockStack = new Stack(); 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)) + bool visitedNew = false; + + for (int i = 0; i < block.SuccessorCount; i++) { - blockStack.Push(block); - blockStack.Push(block.Next); + BasicBlock succ = block.GetSuccessor(i); + + if (visited.Add(succ)) + { + blockStack.Push(block); + blockStack.Push(succ); + + visitedNew = true; + + break; + } } - else if (block.Branch != null && visited.Add(block.Branch)) - { - blockStack.Push(block); - blockStack.Push(block.Branch); - } - else + + if (!visitedNew) { PostOrderMap[block.Index] = index; @@ -59,26 +60,24 @@ namespace ARMeilleure.Translation private void RemoveUnreachableBlocks(IntrusiveList blocks) { - HashSet visited = new HashSet(); - - Queue workQueue = new Queue(); + var visited = new HashSet(); + var workQueue = new Queue(); 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); diff --git a/ARMeilleure/Translation/EmitterContext.cs b/ARMeilleure/Translation/EmitterContext.cs index a6cc55dfb..2261fb877 100644 --- a/ARMeilleure/Translation/EmitterContext.cs +++ b/ARMeilleure/Translation/EmitterContext.cs @@ -13,18 +13,17 @@ namespace ARMeilleure.Translation class EmitterContext { - private Dictionary _irLabels; - - private IntrusiveList _irBlocks; + private readonly Dictionary _irLabels; + private readonly IntrusiveList _irBlocks; private BasicBlock _irBlock; + private BasicBlock _ifBlock; private bool _needsNewBlock; public EmitterContext() { _irLabels = new Dictionary(); - _irBlocks = new IntrusiveList(); _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); } diff --git a/ARMeilleure/Translation/PTC/Ptc.cs b/ARMeilleure/Translation/PTC/Ptc.cs index eebf80759..ae884ab65 100644 --- a/ARMeilleure/Translation/PTC/Ptc.cs +++ b/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"; diff --git a/ARMeilleure/Translation/RegisterUsage.cs b/ARMeilleure/Translation/RegisterUsage.cs index d51242856..6a21ae2a9 100644 --- a/ARMeilleure/Translation/RegisterUsage.cs +++ b/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]; diff --git a/ARMeilleure/Translation/Translator.cs b/ARMeilleure/Translation/Translator.cs index d1404796a..1a02bce0c 100644 --- a/ARMeilleure/Translation/Translator.cs +++ b/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));