Merge branch '#833-correct-data-segment-for-string-constants' into 'master'

#833 - Corrected string constantification using the correct data segment

See merge request camelot/kickc!57

src/main/java/dk/camelot64/kickc/model/Initializers.java

@@ -71,7 +71,7 @@ public class Initializers {
     * @param source The statement source (for error messages)
     * @return The constantified value. A {@link ConstantValue} is possible
     */
-   public static RValue constantify(RValue initValue, ValueTypeSpec typeSpec, Program program, StatementSource source) {
+   public static RValue constantify(RValue initValue, ValueTypeSpec typeSpec, String segmentData, Program program, StatementSource source) {
       // Remove any const/volatile qualifiers
       typeSpec = new ValueTypeSpec(typeSpec.getType().getQualified(false, false));
 
@@ -84,21 +84,21 @@ public class Initializers {
          final CastValue castValue = (CastValue) initValue;
          if(castValue.getValue() instanceof ValueList && castValue.getToType() instanceof SymbolTypeStruct) {
             final SymbolType toType = castValue.getToType();
-            final RValue constantSub = constantify(castValue.getValue(), new ValueTypeSpec(toType), program, source);
+            final RValue constantSub = constantify(castValue.getValue(), new ValueTypeSpec(toType), segmentData, program, source);
             if(constantSub instanceof ConstantValue) {
                return new ConstantCastValue(toType, (ConstantValue) constantSub);
             }
          }
       } else if(initValue instanceof UnionDesignator) {
-         initValue = constantifyUnion((UnionDesignator) initValue, (SymbolTypeStruct) typeSpec.getType(), program, source);
+         initValue = constantifyUnion((UnionDesignator) initValue, (SymbolTypeStruct) typeSpec.getType(), segmentData, program, source);
       } else if(initValue instanceof ValueList) {
          ValueList initList = (ValueList) initValue;
          if(typeSpec.getType() instanceof SymbolTypePointer && ((SymbolTypePointer) typeSpec.getType()).getArraySpec() != null) {
             // Type is an array
-            initValue = constantifyArray(initList, (SymbolTypePointer) typeSpec.getType(), program, source);
+            initValue = constantifyArray(initList, (SymbolTypePointer) typeSpec.getType(), segmentData, program, source);
          } else if(typeSpec.getType() instanceof SymbolTypeStruct) {
             // Type is a struct
-            initValue = constantifyStructOrUnion(initList, (SymbolTypeStruct) typeSpec.getType(), program, source);
+            initValue = constantifyStructOrUnion(initList, (SymbolTypeStruct) typeSpec.getType(), segmentData, program, source);
          } else {
             throw new CompileError("Value list cannot initialize type " + typeSpec.getType(), source);
          }
@@ -138,7 +138,7 @@ public class Initializers {
     * @param source The source line
     * @return The constantified value
     */
-   private static RValue constantifyUnion(UnionDesignator unionInit, SymbolTypeStruct structType, Program program, StatementSource source) {
+   private static RValue constantifyUnion(UnionDesignator unionInit, SymbolTypeStruct structType, String segmentData, Program program, StatementSource source) {
       StructDefinition structDefinition = structType.getStructDefinition(program.getScope());
       Collection<Variable> memberDefinitions = structDefinition.getAllVars(false);
 
@@ -154,7 +154,7 @@ public class Initializers {
       if(memberDef==null)
          throw new CompileError( "Union member not found", source);
 
-      RValue constantifiedMemberValue = constantify(initValue, new ValueTypeSpec(memberDef.getType()), program, source);
+      RValue constantifiedMemberValue = constantify(initValue, new ValueTypeSpec(memberDef.getType()), segmentData, program, source);
       if(constantifiedMemberValue instanceof ConstantValue) {
          LinkedHashMap<SymbolVariableRef, ConstantValue> constMemberMap = new LinkedHashMap<>();
          constMemberMap.put(memberDef.getRef(), (ConstantValue) constantifiedMemberValue);
@@ -173,7 +173,7 @@ public class Initializers {
     * @param source The source line
     * @return The constantified value
     */
-   private static RValue constantifyStructOrUnion(ValueList valueList, SymbolTypeStruct structType, Program program, StatementSource source) {
+   private static RValue constantifyStructOrUnion(ValueList valueList, SymbolTypeStruct structType, String segmentData, Program program, StatementSource source) {
       // Recursively cast all sub-elements
       StructDefinition structDefinition = structType.getStructDefinition(program.getScope());
       Collection<Variable> memberDefinitions = structDefinition.getAllVars(false);
@@ -204,7 +204,7 @@ public class Initializers {
       for(int i = 0; i < structInitNeedSize; i++) {
          Variable memberDef = memberDefIt.next();
          RValue memberValue = valueIt.next();
-         RValue constantifiedMemberValue = constantify(memberValue, new ValueTypeSpec(memberDef.getType()), program, source);
+         RValue constantifiedMemberValue = constantify(memberValue, new ValueTypeSpec(memberDef.getType()), segmentData, program, source);
          constantifiedList.add(constantifiedMemberValue);
          if(constantifiedMemberValue instanceof ConstantValue)
             constMemberMap.put(memberDef.getRef(), (ConstantValue) constantifiedMemberValue);
@@ -229,7 +229,7 @@ public class Initializers {
     * @param source The source line
     * @return The constantified value
     */
-   private static RValue constantifyArray(ValueList valueList, SymbolTypePointer arrayType, Program program, StatementSource source) {
+   private static RValue constantifyArray(ValueList valueList, SymbolTypePointer arrayType, String segmentData, Program program, StatementSource source) {
       ArraySpec arraySpec = arrayType.getArraySpec();
       SymbolType elementType = arrayType.getElementType();
       // TODO: Handle array of array
@@ -237,7 +237,7 @@ public class Initializers {
       boolean allConst = true;
       List<RValue> constantifiedList = new ArrayList<>();
       for(RValue elementValue : valueList.getList()) {
-         RValue constantifiedElement = constantify(elementValue, elementTypeSpec, program, source);
+         RValue constantifiedElement = constantify(elementValue, elementTypeSpec, segmentData, program, source);
          constantifiedList.add(constantifiedElement);
          if(!(constantifiedElement instanceof ConstantValue))
             allConst = false;
@@ -248,7 +248,7 @@ public class Initializers {
          for(RValue rValue : constantifiedList) {
             constElementList.add((ConstantValue) rValue);
          }
-         return new ConstantArrayList(constElementList, elementType, arraySpec.getArraySize());
+         return new ConstantArrayList(constElementList, elementType, arraySpec.getArraySize(), segmentData);
       } else {
          // Convert to a ValueList with a cast to the pointer-type (throwing away the array size)
          return new CastValue(arrayType, new ValueList(constantifiedList));

src/main/java/dk/camelot64/kickc/model/values/ConstantArrayList.java

@@ -22,10 +22,13 @@ public class ConstantArrayList implements ConstantArray {
    /** Array size (from declaration) */
    private ConstantValue size;
 
-   public ConstantArrayList(List<ConstantValue> list, SymbolType elementType, ConstantValue size) {
+   private String segmentData;
+
+   public ConstantArrayList(List<ConstantValue> list, SymbolType elementType, ConstantValue size, String segmentData) {
       this.list = list;
       this.elementType = elementType;
       this.size = size;
+      this.segmentData = segmentData;
    }
 
    @Override
@@ -41,6 +44,10 @@ public class ConstantArrayList implements ConstantArray {
       return list;
    }
 
+   public String getSegmentData() {
+      return segmentData;
+   }
+
    @Override
    public ConstantValue getSize() {
       return size;

src/main/java/dk/camelot64/kickc/passes/Pass0GenerateStatementSequence.java

@@ -1048,7 +1048,7 @@ public class Pass0GenerateStatementSequence extends KickCParserBaseVisitor<Objec
                if(initializer != null)
                   PrePostModifierHandler.addPreModifiers(this, initializer, declSource);
                RValue initValue = (initializer == null) ? null : (RValue) visit(initializer);
-               initValue = Initializers.constantify(initValue, new Initializers.ValueTypeSpec(effectiveType), program, declSource);
+               initValue = Initializers.constantify(initValue, new Initializers.ValueTypeSpec(effectiveType), variable.getDataSegment(), program, declSource);
                boolean isPermanent = ScopeRef.ROOT.equals(variable.getScope().getRef()) || variable.isPermanent();
                if(variable.isKindConstant() || (isPermanent && variable.isKindLoadStore() && Variable.MemoryArea.MAIN_MEMORY.equals(variable.getMemoryArea()) && initValue instanceof ConstantValue && !isStructMember && variable.getRegister() == null)) {
                   // Set initial value

src/main/java/dk/camelot64/kickc/passes/Pass1ExtractInlineStrings.java

@@ -20,93 +20,93 @@ import java.util.List;
  */
 public class Pass1ExtractInlineStrings extends Pass1Base {
 
-   public Pass1ExtractInlineStrings(Program program) {
-      super(program);
-   }
+    public Pass1ExtractInlineStrings(Program program) {
+        super(program);
+    }
 
-   @Override
-   public boolean step() {
-      ProgramValueIterator.execute(getProgram(), (programValue, currentStmt, stmtIt, currentBlock) -> {
-         String nameHint = null;
-         if(currentStmt instanceof StatementCall) {
-            StatementCall call = (StatementCall) currentStmt;
-            List<RValue> parameters = call.getParameters();
-            for(int i = 0; i < parameters.size(); i++) {
-               RValue parameter = parameters.get(i);
-               if(parameter.equals(programValue.get())) {
-                  // The programValue value is the parameter - use the parameter name as name hint
-                  Procedure procedure = Pass1ExtractInlineStrings.this.getProgram().getScope().getProcedure(call.getProcedure());
-                  nameHint = procedure.getParameterNames().get(i);
-                  break;
-               }
+    @Override
+    public boolean step() {
+        ProgramValueIterator.execute(getProgram(), (programValue, currentStmt, stmtIt, currentBlock) -> {
+            String nameHint = null;
+            if (currentStmt instanceof StatementCall) {
+                StatementCall call = (StatementCall) currentStmt;
+                List<RValue> parameters = call.getParameters();
+                for (int i = 0; i < parameters.size(); i++) {
+                    RValue parameter = parameters.get(i);
+                    if (parameter.equals(programValue.get())) {
+                        // The programValue value is the parameter - use the parameter name as name hint
+                        Procedure procedure = Pass1ExtractInlineStrings.this.getProgram().getScope().getProcedure(call.getProcedure());
+                        nameHint = procedure.getParameterNames().get(i);
+                        break;
+                    }
+                }
+            } else if (currentStmt instanceof StatementCallPrepare) {
+                StatementCallPrepare call = (StatementCallPrepare) currentStmt;
+                List<RValue> parameters = call.getParameters();
+                for (int i = 0; i < parameters.size(); i++) {
+                    RValue parameter = parameters.get(i);
+                    if (parameter.equals(programValue.get())) {
+                        // The programValue value is the parameter - use the parameter name as name hint
+                        Procedure procedure = Pass1ExtractInlineStrings.this.getProgram().getScope().getProcedure(call.getProcedure());
+                        nameHint = procedure.getParameterNames().get(i);
+                        break;
+                    }
+                }
             }
-         } else if(currentStmt instanceof StatementCallPrepare) {
-            StatementCallPrepare call = (StatementCallPrepare) currentStmt;
-            List<RValue> parameters = call.getParameters();
-            for(int i = 0; i < parameters.size(); i++) {
-               RValue parameter = parameters.get(i);
-               if(parameter.equals(programValue.get())) {
-                  // The programValue value is the parameter - use the parameter name as name hint
-                  Procedure procedure = Pass1ExtractInlineStrings.this.getProgram().getScope().getProcedure(call.getProcedure());
-                  nameHint = procedure.getParameterNames().get(i);
-                  break;
-               }
+            Value value = programValue.get();
+            if (value instanceof ConstantString && currentStmt != null) {
+                // String in statement expression
+                Scope blockScope = Pass1ExtractInlineStrings.this.getProgram().getScope().getScope(currentBlock.getScope());
+                Variable strConst = Pass1ExtractInlineStrings.this.createStringConstantVar(blockScope, (ConstantString) programValue.get(), nameHint, blockScope.getSegmentData());
+                programValue.set(strConst.getRef());
+            } else if (value instanceof ConstantString && programValue instanceof ProgramValue.ProgramValueConstantStructMember) {
+                // Struct member initialization
+                final ProgramValue.ProgramValueConstantStructMember constantStructMember = (ProgramValue.ProgramValueConstantStructMember) programValue;
+                final SymbolVariableRef memberRef = constantStructMember.getMemberRef();
+                final Variable memberDef = getProgramScope().getVar(memberRef);
+                if (memberDef.getType() instanceof SymbolTypePointer) {
+                    if (((SymbolTypePointer) memberDef.getType()).getArraySpec() == null) {
+                        // Member is not an array - create a string.
+                        nameHint = memberDef.getFullName().replace("::", "_").toLowerCase();
+                        Variable strConst = Pass1ExtractInlineStrings.this.createStringConstantVar(getProgramScope(), (ConstantString) programValue.get(), nameHint, memberDef.getDataSegment());
+                        programValue.set(strConst.getRef());
+                    }
+                }
+            } else if (value instanceof ConstantString && programValue instanceof ProgramValue.ProgramValueConstantArrayElement) {
+                // Array element initialization
+                final ProgramValue.ProgramValueConstantArrayElement constantArrayElement = (ProgramValue.ProgramValueConstantArrayElement) programValue;
+                final SymbolType elementType = constantArrayElement.getArrayList().getElementType();
+                if (elementType instanceof SymbolTypePointer) {
+                    if (((SymbolTypePointer) elementType).getArraySpec() == null) {
+                        // Element is not an array - create a string.
+                        Variable strConst = Pass1ExtractInlineStrings.this.createStringConstantVar(getProgramScope(), (ConstantString) programValue.get(), null, constantArrayElement.getArrayList().getSegmentData());
+                        programValue.set(strConst.getRef());
+                    }
+                }
             }
-         }
-         Value value = programValue.get();
-         if(value instanceof ConstantString && currentStmt!=null) {
-            // String in statement expression
-            Scope blockScope = Pass1ExtractInlineStrings.this.getProgram().getScope().getScope(currentBlock.getScope());
-            Variable strConst = Pass1ExtractInlineStrings.this.createStringConstantVar(blockScope, (ConstantString) programValue.get(), nameHint);
-            programValue.set(strConst.getRef());
-         } else if(value instanceof ConstantString && programValue instanceof ProgramValue.ProgramValueConstantStructMember) {
-            // Struct member initialization
-            final ProgramValue.ProgramValueConstantStructMember constantStructMember = (ProgramValue.ProgramValueConstantStructMember) programValue;
-            final SymbolVariableRef memberRef = constantStructMember.getMemberRef();
-            final Variable memberDef = getProgramScope().getVar(memberRef);
-            if(memberDef.getType() instanceof SymbolTypePointer) {
-               if(((SymbolTypePointer) memberDef.getType()).getArraySpec()==null) {
-                  // Member is not an array - create a string.
-                  nameHint = memberDef.getFullName().replace("::","_").toLowerCase();
-                  Variable strConst = Pass1ExtractInlineStrings.this.createStringConstantVar(getProgramScope(), (ConstantString) programValue.get(), nameHint);
-                  programValue.set(strConst.getRef());
-               }
-            }
-      } else if(value instanceof ConstantString && programValue instanceof ProgramValue.ProgramValueConstantArrayElement) {
-            // Array element initialization
-            final ProgramValue.ProgramValueConstantArrayElement constantArrayElement = (ProgramValue.ProgramValueConstantArrayElement) programValue;
-            final SymbolType elementType = constantArrayElement.getArrayList().getElementType();
-            if(elementType instanceof SymbolTypePointer) {
-               if(((SymbolTypePointer) elementType).getArraySpec()==null) {
-                  // Element is not an array - create a string.
-                  Variable strConst = Pass1ExtractInlineStrings.this.createStringConstantVar(getProgramScope(), (ConstantString) programValue.get(), null);
-                  programValue.set(strConst.getRef());
-               }
-            }
-         }
-      });
-      return false;
-   }
+        });
+        return false;
+    }
 
-   private Variable createStringConstantVar(Scope blockScope, ConstantString constantString, String nameHint) {
-      String name;
-      if(nameHint == null) {
-         name = blockScope.allocateIntermediateVariableName();
-      } else {
-         int nameHintIdx = 1;
-         name = nameHint;
-         while(blockScope.getLocalSymbol(name) != null) {
-            name = nameHint + nameHintIdx++;
-         }
-      }
-      final long stringLength = constantString.getStringLength();
-      final ConstantInteger arraySize = new ConstantInteger(stringLength, stringLength<256?SymbolType.BYTE : SymbolType.WORD);
-      Variable strConst = Variable.createConstant(name, new SymbolTypePointer(SymbolType.BYTE, new ArraySpec(arraySize), false, false), blockScope, constantString, blockScope.getSegmentData());
-      blockScope.add(strConst);
-      if(getLog().isVerbosePass1CreateSsa()) {
-         getLog().append("Creating constant string variable for inline " + strConst.toString(getProgram()) + " \"" + constantString.getValue() + "\"");
-      }
-      return strConst;
-   }
+    private Variable createStringConstantVar(Scope blockScope, ConstantString constantString, String nameHint, String segmentData) {
+        String name;
+        if (nameHint == null) {
+            name = blockScope.allocateIntermediateVariableName();
+        } else {
+            int nameHintIdx = 1;
+            name = nameHint;
+            while (blockScope.getLocalSymbol(name) != null) {
+                name = nameHint + nameHintIdx++;
+            }
+        }
+        final long stringLength = constantString.getStringLength();
+        final ConstantInteger arraySize = new ConstantInteger(stringLength, stringLength < 256 ? SymbolType.BYTE : SymbolType.WORD);
+        Variable strConst = Variable.createConstant(name, new SymbolTypePointer(SymbolType.BYTE, new ArraySpec(arraySize), false, false), blockScope, constantString, segmentData);
+        blockScope.add(strConst);
+        if (getLog().isVerbosePass1CreateSsa()) {
+            getLog().append("Creating constant string variable for inline " + strConst.toString(getProgram()) + " \"" + constantString.getValue() + "\"");
+        }
+        return strConst;
+    }
 
 }

src/main/java/dk/camelot64/kickc/passes/Pass1UnwindStructPrepare.java

@@ -36,7 +36,7 @@ public class Pass1UnwindStructPrepare extends Pass2SsaOptimization {
                if(!(rValue instanceof ConstantValue) && lValueType instanceof SymbolTypeStruct) {
                   // TODO: Constantify all R-Values?
                   Initializers.ValueTypeSpec lValueTypeSpec = new Initializers.ValueTypeSpec(lValueType);
-                  RValue rValueConstantified = Initializers.constantify(rValue, lValueTypeSpec, getProgram(), assignment.getSource());
+                  RValue rValueConstantified = Initializers.constantify(rValue, lValueTypeSpec, getProgram().getScope().getSegmentData(), getProgram(), assignment.getSource());
                   if(!rValue.equals(rValueConstantified)) {
                      assignment.setrValue2(rValueConstantified);
                      getLog().append("Constantified RValue "+assignment.toString(getProgram(), false));

src/test/java/dk/camelot64/kickc/test/TestProgramsFast.java

@@ -4039,6 +4039,11 @@ public class TestProgramsFast extends TestPrograms {
       compileAndCompare("string-const-consolidation.c");
    }
 
+   @Test
+   public void testStringConstConsolidationSegmented() throws IOException {
+      compileAndCompare("string-const-consolidation-segmented.c");
+   }
+
    @Test
    public void testStringConstConsolidationRoot() throws IOException {
       compileAndCompare("string-const-consolidation-root.c");

src/test/kc/string-const-consolidation-segmented.c

@@ -0,0 +1,13 @@
+// Tests that identical strings are consolidated
+
+#pragma link("string-const-link.ld")
+#pragma data_seg(StringTest)
+
+__export char* test[2] = {"hello", "world"};
+
+
+void main() {
+
+
+
+}

src/test/kc/string-const-link.ld

@@ -0,0 +1,3 @@
+.segmentdef StringTest
+.segmentdef Data
+.segmentdef Code

src/test/ref/string-const-consolidation-segmented.asm

@@ -0,0 +1,14 @@
+// Tests that identical strings are consolidated
+  .segmentdef StringTest
+.segmentdef Data
+.segmentdef Code
+.segment Code
+main: {
+    rts
+}
+.segment StringTest
+  test: .word __0, __1
+  __0: .text "hello"
+  .byte 0
+  __1: .text "world"
+  .byte 0

src/test/ref/string-const-consolidation-segmented.cfg

@@ -0,0 +1,8 @@
+
+void main()
+main: scope:[main]  from
+  [0] phi()
+  to:main::@return
+main::@return: scope:[main]  from main
+  [1] return 
+  to:@return

src/test/ref/string-const-consolidation-segmented.log

@@ -0,0 +1,131 @@
+Loading link script "string-const-link.ld"
+
+CONTROL FLOW GRAPH SSA
+
+void main()
+main: scope:[main]  from __start
+  to:main::@return
+main::@return: scope:[main]  from main
+  return 
+  to:@return
+
+void __start()
+__start: scope:[__start]  from
+  call main
+  to:__start::@1
+__start::@1: scope:[__start]  from __start
+  to:__start::@return
+__start::@return: scope:[__start]  from __start::@1
+  return 
+  to:@return
+
+SYMBOL TABLE SSA
+__constant char $0[6] = "hello"
+__constant char $1[6] = "world"
+void __start()
+void main()
+__constant char *test[2] = { $0, $1 }
+
+Removing unused procedure __start
+Removing unused procedure block __start
+Removing unused procedure block __start::@1
+Removing unused procedure block __start::@return
+Successful SSA optimization PassNEliminateEmptyStart
+Finalized unsigned number type (char) 2
+Successful SSA optimization PassNFinalizeNumberTypeConversions
+Adding NOP phi() at start of main
+CALL GRAPH
+
+Created 0 initial phi equivalence classes
+Coalesced down to 0 phi equivalence classes
+Adding NOP phi() at start of main
+
+FINAL CONTROL FLOW GRAPH
+
+void main()
+main: scope:[main]  from
+  [0] phi()
+  to:main::@return
+main::@return: scope:[main]  from main
+  [1] return 
+  to:@return
+
+
+VARIABLE REGISTER WEIGHTS
+void main()
+
+Initial phi equivalence classes
+Complete equivalence classes
+REGISTER UPLIFT POTENTIAL REGISTERS
+
+REGISTER UPLIFT SCOPES
+Uplift Scope [main] 
+Uplift Scope [] 
+
+Uplifting [main] best 36 combination 
+Uplifting [] best 36 combination 
+
+ASSEMBLER BEFORE OPTIMIZATION
+  // File Comments
+// Tests that identical strings are consolidated
+  // Upstart
+  .segmentdef StringTest
+.segmentdef Data
+.segmentdef Code
+  // Global Constants & labels
+.segment Code
+  // main
+main: {
+    jmp __breturn
+    // main::@return
+  __breturn:
+    // [1] return 
+    rts
+}
+  // File Data
+.segment StringTest
+  test: .word __0, __1
+  __0: .text "hello"
+  .byte 0
+  __1: .text "world"
+  .byte 0
+
+ASSEMBLER OPTIMIZATIONS
+Removing instruction jmp __breturn
+Succesful ASM optimization Pass5NextJumpElimination
+Removing instruction __breturn:
+Succesful ASM optimization Pass5UnusedLabelElimination
+
+FINAL SYMBOL TABLE
+__constant char $0[6] = "hello"
+__constant char $1[6] = "world"
+void main()
+__constant char *test[2] = { $0, $1 }
+
+
+
+FINAL ASSEMBLER
+Score: 6
+
+  // File Comments
+// Tests that identical strings are consolidated
+  // Upstart
+  .segmentdef StringTest
+.segmentdef Data
+.segmentdef Code
+  // Global Constants & labels
+.segment Code
+  // main
+main: {
+    // main::@return
+    // [1] return 
+    rts
+}
+  // File Data
+.segment StringTest
+  test: .word __0, __1
+  __0: .text "hello"
+  .byte 0
+  __1: .text "world"
+  .byte 0
+

src/test/ref/string-const-consolidation-segmented.sym

@@ -0,0 +1,5 @@
+__constant char $0[6] = "hello"
+__constant char $1[6] = "world"
+void main()
+__constant char *test[2] = { $0, $1 }
+