From 19b145c2079f0715153a8479c589c407917b8c17 Mon Sep 17 00:00:00 2001 From: jespergravgaard Date: Sun, 23 Feb 2020 20:57:53 +0100 Subject: [PATCH] Implemented immediate variables for preserving the value of load/store variables used in simple conditions that are inlined. #359 --- .../kickc/model/ControlFlowBlock.java | 20 ++- .../kickc/model/ControlFlowGraph.java | 2 +- .../Pass2ConditionalJumpSimplification.java | 127 +++++++++++++----- .../kickc/passes/utils/Unroller.java | 6 +- src/test/ref/varmodel-ma_mem-5.asm | 8 +- src/test/ref/varmodel-ma_mem-5.cfg | 4 +- src/test/ref/varmodel-ma_mem-5.log | 76 +++++------ src/test/ref/varmodel-ma_mem-5.sym | 4 +- 8 files changed, 155 insertions(+), 92 deletions(-) diff --git a/src/main/java/dk/camelot64/kickc/model/ControlFlowBlock.java b/src/main/java/dk/camelot64/kickc/model/ControlFlowBlock.java index d0032d239..a1d4e1e4a 100644 --- a/src/main/java/dk/camelot64/kickc/model/ControlFlowBlock.java +++ b/src/main/java/dk/camelot64/kickc/model/ControlFlowBlock.java @@ -92,7 +92,25 @@ public class ControlFlowBlock implements Serializable { return; } } - throw new RuntimeException("No call statement in block " + getLabel().getFullName()); + throw new InternalError("No call statement in block " + getLabel().getFullName()); + } + + /** + * Adds a new statement after an existing predecessor statement + * @param newStatement The new statement to add + * @param predecessor The existing predecessor statement + */ + public void addStatementAfter(Statement newStatement, Statement predecessor) { + ListIterator listIterator = statements.listIterator(); + while(listIterator.hasNext()) { + Statement statement = listIterator.next(); + if(statement.equals(predecessor)) { + listIterator.previous(); + listIterator.add(newStatement); + return; + } + } + throw new InternalError("Predecessor not found in block " +getLabel().getFullName() + " predecessor: "+ predecessor.toString()); } public LabelRef getDefaultSuccessor() { diff --git a/src/main/java/dk/camelot64/kickc/model/ControlFlowGraph.java b/src/main/java/dk/camelot64/kickc/model/ControlFlowGraph.java index 879271976..5e60388ae 100644 --- a/src/main/java/dk/camelot64/kickc/model/ControlFlowGraph.java +++ b/src/main/java/dk/camelot64/kickc/model/ControlFlowGraph.java @@ -199,7 +199,7 @@ public class ControlFlowGraph implements Serializable { public Statement getStatementByIndex(int statementIdx) { for(ControlFlowBlock block : getAllBlocks()) { for(Statement statement : block.getStatements()) { - if(statementIdx == statement.getIndex()) { + if(statement.getIndex()!=null && statementIdx == statement.getIndex()) { return statement; } } diff --git a/src/main/java/dk/camelot64/kickc/passes/Pass2ConditionalJumpSimplification.java b/src/main/java/dk/camelot64/kickc/passes/Pass2ConditionalJumpSimplification.java index 28ebbfca3..5999f62b6 100644 --- a/src/main/java/dk/camelot64/kickc/passes/Pass2ConditionalJumpSimplification.java +++ b/src/main/java/dk/camelot64/kickc/passes/Pass2ConditionalJumpSimplification.java @@ -1,18 +1,27 @@ package dk.camelot64.kickc.passes; +import dk.camelot64.kickc.model.Comment; import dk.camelot64.kickc.model.ControlFlowBlock; import dk.camelot64.kickc.model.Program; import dk.camelot64.kickc.model.VariableReferenceInfos; +import dk.camelot64.kickc.model.iterator.ProgramValue; +import dk.camelot64.kickc.model.iterator.ProgramValueIterator; +import dk.camelot64.kickc.model.operators.Operator; import dk.camelot64.kickc.model.statements.Statement; import dk.camelot64.kickc.model.statements.StatementAssignment; import dk.camelot64.kickc.model.statements.StatementConditionalJump; import dk.camelot64.kickc.model.statements.StatementInfos; +import dk.camelot64.kickc.model.symbols.Scope; import dk.camelot64.kickc.model.symbols.Variable; import dk.camelot64.kickc.model.values.RValue; +import dk.camelot64.kickc.model.values.ScopeRef; +import dk.camelot64.kickc.model.values.SymbolRef; import dk.camelot64.kickc.model.values.VariableRef; +import dk.camelot64.kickc.passes.utils.AliasReplacer; import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedHashMap; import java.util.List; import java.util.stream.Collectors; @@ -27,15 +36,38 @@ public class Pass2ConditionalJumpSimplification extends Pass2SsaOptimization { @Override public boolean step() { - final List simpleConditionVars = getSimpleConditions(); + final List simpleConditionVars = new ArrayList<>(); + List simpleConditions = getSimpleConditionTodos(); + for(SimpleCondition simpleCondition : simpleConditions) { + rewriteSimpleCondition(simpleCondition); + simpleConditionVars.add(simpleCondition.conditionVar); + } removeAssignments(getGraph(), simpleConditionVars); deleteSymbols(getScope(), simpleConditionVars); return (simpleConditionVars.size() > 0); } - private List getSimpleConditions() { + /** An identified conditional jump with a one-variable condition that uses a comparison condition (less-than, greater-than, ...) and the assignment for that condition. */ + static class SimpleCondition { + StatementConditionalJump conditionalJump; + StatementAssignment conditionAssignment; + VariableRef conditionVar; + + SimpleCondition(StatementConditionalJump conditionalJump, StatementAssignment conditionAssignment, VariableRef conditionVar) { + this.conditionalJump = conditionalJump; + this.conditionAssignment = conditionAssignment; + this.conditionVar = conditionVar; + } + } + + /** + * Find all simple conditions that can be rewritten. + * Simple conditions are conditional jumps with a single variable as condition. The condition must be the result of an assignment with a comparison-operator. + * @return The simple conditions to rewrite + */ + private List getSimpleConditionTodos() { final VariableReferenceInfos variableReferenceInfos = getProgram().getVariableReferenceInfos(); - final List simpleConditionVars = new ArrayList<>(); + List todos = new ArrayList<>(); for(ControlFlowBlock block : getGraph().getAllBlocks()) { for(Statement statement : block.getStatements()) { if(statement instanceof StatementConditionalJump) { @@ -60,35 +92,7 @@ public class Pass2ConditionalJumpSimplification extends Pass2SsaOptimization { case "=<": case ">=": case "=>": - final Collection referencedLoadStoreVariables = getReferencedLoadStoreVariables(conditionAssignment.getrValue1()); - referencedLoadStoreVariables.addAll(getReferencedLoadStoreVariables(conditionAssignment.getrValue2())); - boolean isSimple = true; - if(referencedLoadStoreVariables.size() > 0) { - // Found referenced load/store variables - // Examine all statements between the conditionAssignment and conditionalJump for modifications - final StatementInfos statementInfos = getProgram().getStatementInfos(); - Collection statementsBetween = getGraph().getStatementsBetween(conditionAssignment, conditionalJump, statementInfos); - for(Statement statementBetween : statementsBetween) { - for(Variable referencedLoadStoreVariable : referencedLoadStoreVariables) { - if(variableReferenceInfos.getDefinedVars(statementBetween).contains(referencedLoadStoreVariable.getVariableRef())) { - // A referenced load/store-variable is modified in a statement between the assignment and the condition! - isSimple = false; - getLog().append("Condition not simple " + conditionVar.toString(getProgram()) + " " + conditionalJump.toString(getProgram(), false)); - // TODO: Introduce intermediate variable copy of the load/store-variable and use that in the condition! - - } - } - } - } - - if(isSimple) { - conditionalJump.setrValue1(conditionAssignment.getrValue1()); - conditionalJump.setOperator(conditionAssignment.getOperator()); - conditionalJump.setrValue2(conditionAssignment.getrValue2()); - simpleConditionVars.add(conditionVar); - getLog().append("Simple Condition " + conditionVar.toString(getProgram()) + " " + conditionalJump.toString(getProgram(), false)); - break; - } + todos.add(new SimpleCondition(conditionalJump, conditionAssignment, conditionVar)); default: } } @@ -98,7 +102,64 @@ public class Pass2ConditionalJumpSimplification extends Pass2SsaOptimization { } } } - return simpleConditionVars; + return todos; + } + + /** + * Rewrite a simple condition - by inlining the comparison into the conditional jump. + * If the condition uses any load/store-variables that are modified between the condition assignment and the jump the values of these is preserved in a new intermediate variable. + * + * @param simpleCondition The simple condition to rewrite + */ + private void rewriteSimpleCondition(SimpleCondition simpleCondition) { + final VariableReferenceInfos variableReferenceInfos = getProgram().getVariableReferenceInfos(); + // For each condition/assignment pair - do a rewrite to inline the condition if possible + final Operator operator = simpleCondition.conditionAssignment.getOperator(); + RValue rValue1 = simpleCondition.conditionAssignment.getrValue1(); + RValue rValue2 = simpleCondition.conditionAssignment.getrValue2(); + final Collection referencedLoadStoreVariables = getReferencedLoadStoreVariables(rValue1); + referencedLoadStoreVariables.addAll(getReferencedLoadStoreVariables(rValue2)); + if(referencedLoadStoreVariables.size() > 0) { + // Found referenced load/store variables + // Examine all statements between the conditionAssignment and conditionalJump for modifications + final StatementInfos statementInfos = getProgram().getStatementInfos(); + Collection statementsBetween = getGraph().getStatementsBetween(simpleCondition.conditionAssignment, simpleCondition.conditionalJump, statementInfos); + for(Statement statementBetween : statementsBetween) { + for(Variable referencedLoadStoreVariable : referencedLoadStoreVariables) { + if(variableReferenceInfos.getDefinedVars(statementBetween).contains(referencedLoadStoreVariable.getVariableRef())) { + // A referenced load/store-variable is modified in a statement between the assignment and the condition! + getLog().append("Condition not simple " + simpleCondition.conditionVar.toString(getProgram()) + " " + simpleCondition.conditionalJump.toString(getProgram(), false)); + // Create an intermediate variable copy of the load/store-variable at the position of the condition-variable to preserve the value + final ControlFlowBlock conditionDefineBlock = statementInfos.getBlock(simpleCondition.conditionAssignment); + final ScopeRef conditionDefineScopeRef = conditionDefineBlock.getScope(); + final Scope conditionDefineScope = getScope().getScope(conditionDefineScopeRef); + final Variable intermediateLoadStoreVar = conditionDefineScope.addVariableIntermediate(); + intermediateLoadStoreVar.setType(referencedLoadStoreVariable.getType()); + final StatementAssignment intermediateLoadStoreAssignment = new StatementAssignment(intermediateLoadStoreVar.getVariableRef(), referencedLoadStoreVariable.getRef(), true, simpleCondition.conditionAssignment.getSource(), Comment.NO_COMMENTS); + conditionDefineBlock.addStatementAfter(intermediateLoadStoreAssignment, simpleCondition.conditionAssignment); + // Replace all references to the load/store variable in the expressions with the new intermediate + final LinkedHashMap aliases = new LinkedHashMap<>(); + aliases.put(referencedLoadStoreVariable.getRef(), intermediateLoadStoreVar.getRef()); + { + final ProgramValue.GenericValue programValue1 = new ProgramValue.GenericValue(rValue1); + ProgramValueIterator.execute(programValue1, new AliasReplacer(aliases), null, null, null); + rValue1 = (RValue) programValue1.get(); + } + { + final ProgramValue.GenericValue programValue2 = new ProgramValue.GenericValue(rValue2); + ProgramValueIterator.execute(programValue2, new AliasReplacer(aliases), null, null, null); + rValue2 = (RValue) programValue2.get(); + } + getLog().append("Introduced intermediate condition variable " + intermediateLoadStoreAssignment.toString(getProgram(), false)); + } + } + } + } + // Perform the condition rewrite + simpleCondition.conditionalJump.setrValue1(rValue1); + simpleCondition.conditionalJump.setOperator(operator); + simpleCondition.conditionalJump.setrValue2(rValue2); + getLog().append("Simple Condition " + simpleCondition.conditionVar.toString(getProgram()) + " " + simpleCondition.conditionalJump.toString(getProgram(), false)); } /** diff --git a/src/main/java/dk/camelot64/kickc/passes/utils/Unroller.java b/src/main/java/dk/camelot64/kickc/passes/utils/Unroller.java index 816929ad9..ab85cd545 100644 --- a/src/main/java/dk/camelot64/kickc/passes/utils/Unroller.java +++ b/src/main/java/dk/camelot64/kickc/passes/utils/Unroller.java @@ -475,7 +475,7 @@ public class Unroller { RValue rValueNew = valueToNew(origPhiRValue.getrValue(), varsOriginalToCopied); newPhiVariable.setrValue(blocksOriginalToCopied.get(predecessor), rValueNew); // - Then an entry from the existing predecessor block - RValue rValue = valueToOrig(origPhiRValue.getrValue()); + RValue rValue = copyValue(origPhiRValue.getrValue()); newPhiVariable.setrValue(predecessor, rValue); // Finally remove the phi entry into the original block (since both will hit the new block) origPhiRValuesIt.remove(); @@ -530,7 +530,7 @@ public class Unroller { */ private static RValue valueToNew(RValue rValue, Map definedToNewVar) { if(rValue == null) return null; - RValue rValueCopy = valueToOrig(rValue); + RValue rValueCopy = copyValue(rValue); ProgramValue.GenericValue genericValue = new ProgramValue.GenericValue(rValueCopy); ProgramValueIterator.execute(genericValue, (programValue, currentStmt, stmtIt, currentBlock) -> { Value rVal = programValue.get(); @@ -550,7 +550,7 @@ public class Unroller { * @param rValue The value to copy * @return An exact copy of the value */ - private static RValue valueToOrig(RValue rValue) { + public static RValue copyValue(RValue rValue) { if(rValue == null) return null; ProgramValue.GenericValue genericValue = new ProgramValue.GenericValue(rValue); ProgramValueIterator.execute(genericValue, (programValue, currentStmt, stmtIt, currentBlock) -> { diff --git a/src/test/ref/varmodel-ma_mem-5.asm b/src/test/ref/varmodel-ma_mem-5.asm index c4d947724..bfebc4c9a 100644 --- a/src/test/ref/varmodel-ma_mem-5.asm +++ b/src/test/ref/varmodel-ma_mem-5.asm @@ -15,14 +15,10 @@ main: { sta SCREEN,y // i++<4 tya - cmp #4 - lda #0 - rol - eor #1 // while(i++<4) inc i - cmp #0 - bne __b1 + cmp #4 + bcc __b1 // } rts i: .byte 0 diff --git a/src/test/ref/varmodel-ma_mem-5.cfg b/src/test/ref/varmodel-ma_mem-5.cfg index e7ccf6969..badd99214 100644 --- a/src/test/ref/varmodel-ma_mem-5.cfg +++ b/src/test/ref/varmodel-ma_mem-5.cfg @@ -14,9 +14,9 @@ main: scope:[main] from @1 to:main::@1 main::@1: scope:[main] from main main::@1 [5] *((const byte*) SCREEN + (byte) main::i) ← (byte) '*' - [6] (bool~) main::$0 ← (byte) main::i < (byte) 4 + [6] (byte~) main::$1 ← (byte) main::i [7] (byte) main::i ← ++ (byte) main::i - [8] if((bool~) main::$0) goto main::@1 + [8] if((byte~) main::$1<(byte) 4) goto main::@1 to:main::@return main::@return: scope:[main] from main::@1 [9] return diff --git a/src/test/ref/varmodel-ma_mem-5.log b/src/test/ref/varmodel-ma_mem-5.log index f16caa0f9..19e26524f 100644 --- a/src/test/ref/varmodel-ma_mem-5.log +++ b/src/test/ref/varmodel-ma_mem-5.log @@ -44,8 +44,9 @@ Successful SSA optimization PassNCastSimplification Finalized unsigned number type (byte) 4 Successful SSA optimization PassNFinalizeNumberTypeConversions Condition not simple (bool~) main::$0 [4] if((bool~) main::$0) goto main::@1 -Condition not simple (bool~) main::$0 [4] if((bool~) main::$0) goto main::@1 -Condition not simple (bool~) main::$0 [4] if((bool~) main::$0) goto main::@1 +Introduced intermediate condition variable (byte~) main::$1 ← (byte) main::i +Simple Condition (bool~) main::$0 [4] if((byte~) main::$1<(byte) 4) goto main::@1 +Successful SSA optimization Pass2ConditionalJumpSimplification Adding NOP phi() at start of @begin Adding NOP phi() at start of @1 Adding NOP phi() at start of @2 @@ -77,9 +78,9 @@ main: scope:[main] from @1 to:main::@1 main::@1: scope:[main] from main main::@1 [5] *((const byte*) SCREEN + (byte) main::i) ← (byte) '*' - [6] (bool~) main::$0 ← (byte) main::i < (byte) 4 + [6] (byte~) main::$1 ← (byte) main::i [7] (byte) main::i ← ++ (byte) main::i - [8] if((bool~) main::$0) goto main::@1 + [8] if((byte~) main::$1<(byte) 4) goto main::@1 to:main::@return main::@return: scope:[main] from main::@1 [9] return @@ -88,17 +89,17 @@ main::@return: scope:[main] from main::@1 VARIABLE REGISTER WEIGHTS (void()) main() -(bool~) main::$0 11.0 +(byte~) main::$1 11.0 (byte) main::i loadstore 9.200000000000001 Initial phi equivalence classes Added variable main::i to live range equivalence class [ main::i ] -Added variable main::$0 to live range equivalence class [ main::$0 ] +Added variable main::$1 to live range equivalence class [ main::$1 ] Complete equivalence classes [ main::i ] -[ main::$0 ] +[ main::$1 ] Allocated mem[1] [ main::i ] -Allocated zp[1]:2 [ main::$0 ] +Allocated zp[1]:2 [ main::$1 ] INITIAL ASM Target platform is c64basic / MOS6502X @@ -127,7 +128,7 @@ __bend_from___b1: __bend: // main main: { - .label __0 = 2 + .label __1 = 2 // [4] (byte) main::i ← (byte) 0 -- vbum1=vbuc1 lda #0 sta i @@ -138,19 +139,15 @@ main: { lda #'*' ldy i sta SCREEN,y - // [6] (bool~) main::$0 ← (byte) main::i < (byte) 4 -- vboz1=vbum2_lt_vbuc1 + // [6] (byte~) main::$1 ← (byte) main::i -- vbuz1=vbum2 lda i - cmp #4 - lda #0 - rol - eor #1 - sta.z __0 + sta.z __1 // [7] (byte) main::i ← ++ (byte) main::i -- vbum1=_inc_vbum1 inc i - // [8] if((bool~) main::$0) goto main::@1 -- vboz1_then_la1 - lda.z __0 - cmp #0 - bne __b1 + // [8] if((byte~) main::$1<(byte) 4) goto main::@1 -- vbuz1_lt_vbuc1_then_la1 + lda.z __1 + cmp #4 + bcc __b1 jmp __breturn // main::@return __breturn: @@ -163,18 +160,17 @@ main: { REGISTER UPLIFT POTENTIAL REGISTERS Statement [4] (byte) main::i ← (byte) 0 [ main::i ] ( main:2 [ main::i ] ) always clobbers reg byte a Statement [5] *((const byte*) SCREEN + (byte) main::i) ← (byte) '*' [ main::i ] ( main:2 [ main::i ] ) always clobbers reg byte a reg byte y -Statement [6] (bool~) main::$0 ← (byte) main::i < (byte) 4 [ main::i main::$0 ] ( main:2 [ main::i main::$0 ] ) always clobbers reg byte a Potential registers mem[1] [ main::i ] : mem[1] , -Potential registers zp[1]:2 [ main::$0 ] : zp[1]:2 , reg byte a , reg byte x , reg byte y , +Potential registers zp[1]:2 [ main::$1 ] : zp[1]:2 , reg byte a , reg byte x , reg byte y , REGISTER UPLIFT SCOPES -Uplift Scope [main] 11: zp[1]:2 [ main::$0 ] 9.2: mem[1] [ main::i ] +Uplift Scope [main] 11: zp[1]:2 [ main::$1 ] 9.2: mem[1] [ main::i ] Uplift Scope [] -Uplifting [main] best 392 combination reg byte a [ main::$0 ] mem[1] [ main::i ] -Uplifting [] best 392 combination +Uplifting [main] best 312 combination reg byte a [ main::$1 ] mem[1] [ main::i ] +Uplifting [] best 312 combination Attempting to uplift remaining variables inmem[1] [ main::i ] -Uplifting [main] best 392 combination mem[1] [ main::i ] +Uplifting [main] best 312 combination mem[1] [ main::i ] ASSEMBLER BEFORE OPTIMIZATION // File Comments @@ -212,17 +208,13 @@ main: { lda #'*' ldy i sta SCREEN,y - // [6] (bool~) main::$0 ← (byte) main::i < (byte) 4 -- vboaa=vbum1_lt_vbuc1 + // [6] (byte~) main::$1 ← (byte) main::i -- vbuaa=vbum1 lda i - cmp #4 - lda #0 - rol - eor #1 // [7] (byte) main::i ← ++ (byte) main::i -- vbum1=_inc_vbum1 inc i - // [8] if((bool~) main::$0) goto main::@1 -- vboaa_then_la1 - cmp #0 - bne __b1 + // [8] if((byte~) main::$1<(byte) 4) goto main::@1 -- vbuaa_lt_vbuc1_then_la1 + cmp #4 + bcc __b1 jmp __breturn // main::@return __breturn: @@ -259,17 +251,17 @@ FINAL SYMBOL TABLE (label) @end (const byte*) SCREEN = (byte*) 1024 (void()) main() -(bool~) main::$0 reg byte a 11.0 +(byte~) main::$1 reg byte a 11.0 (label) main::@1 (label) main::@return (byte) main::i loadstore mem[1] 9.200000000000001 mem[1] [ main::i ] -reg byte a [ main::$0 ] +reg byte a [ main::$1 ] FINAL ASSEMBLER -Score: 327 +Score: 247 // File Comments // Test memory model @@ -300,18 +292,14 @@ main: { ldy i sta SCREEN,y // i++<4 - // [6] (bool~) main::$0 ← (byte) main::i < (byte) 4 -- vboaa=vbum1_lt_vbuc1 + // [6] (byte~) main::$1 ← (byte) main::i -- vbuaa=vbum1 tya - cmp #4 - lda #0 - rol - eor #1 // while(i++<4) // [7] (byte) main::i ← ++ (byte) main::i -- vbum1=_inc_vbum1 inc i - // [8] if((bool~) main::$0) goto main::@1 -- vboaa_then_la1 - cmp #0 - bne __b1 + // [8] if((byte~) main::$1<(byte) 4) goto main::@1 -- vbuaa_lt_vbuc1_then_la1 + cmp #4 + bcc __b1 // main::@return // } // [9] return diff --git a/src/test/ref/varmodel-ma_mem-5.sym b/src/test/ref/varmodel-ma_mem-5.sym index 4899e3088..cc8791f7f 100644 --- a/src/test/ref/varmodel-ma_mem-5.sym +++ b/src/test/ref/varmodel-ma_mem-5.sym @@ -3,10 +3,10 @@ (label) @end (const byte*) SCREEN = (byte*) 1024 (void()) main() -(bool~) main::$0 reg byte a 11.0 +(byte~) main::$1 reg byte a 11.0 (label) main::@1 (label) main::@return (byte) main::i loadstore mem[1] 9.200000000000001 mem[1] [ main::i ] -reg byte a [ main::$0 ] +reg byte a [ main::$1 ]