Fix inline local declaration codegen

src/main/java/com/aparapi/internal/writer/BlockWriter.java

@@ -89,6 +89,40 @@ public void writeln(String _string) {
 
    public int indent = 0;
 
+   private final Stack<Set<LocalVariableKey>> localDeclarationScopes = new Stack<Set<LocalVariableKey>>();
+
+   private static final class LocalVariableKey{
+      private final int index;
+
+      private final String name;
+
+      private final String descriptor;
+
+      private LocalVariableKey(LocalVariableInfo localVariableInfo) {
+         index = localVariableInfo.getVariableIndex();
+         name = localVariableInfo.getVariableName();
+         descriptor = localVariableInfo.getVariableDescriptor();
+      }
+
+      @Override public boolean equals(Object other) {
+         if (this == other) {
+            return true;
+         }
+         if (!(other instanceof LocalVariableKey)) {
+            return false;
+         }
+         LocalVariableKey otherKey = (LocalVariableKey) other;
+         return (index == otherKey.index) && name.equals(otherKey.name) && descriptor.equals(otherKey.descriptor);
+      }
+
+      @Override public int hashCode() {
+         int result = index;
+         result = (31 * result) + name.hashCode();
+         result = (31 * result) + descriptor.hashCode();
+         return (result);
+      }
+   }
+
    public void in() {
       indent++;
    }
@@ -173,14 +207,14 @@ public void writeComposite(CompositeInstruction instruction) throws CodeGenExcep
          write(")");
          writeBlock(blockStart, null);
       } else if (instruction instanceof CompositeIfElseInstruction) {
+         final Instruction blockStart = instruction.getBranchSet().getLast().getNextExpr();
+         final Instruction elseGoto = getElseGoto(blockStart);
+         writeHoistedLocalDeclarations(blockStart, elseGoto, elseGoto.getNextExpr(), null);
+
          newLine();
          write("if (");
-         final Instruction blockStart = writeConditional(instruction.getBranchSet());
+         writeConditional(instruction.getBranchSet());
          write(")");
-         Instruction elseGoto = blockStart;
-         while (!(elseGoto.isBranch() && elseGoto.asBranch().isUnconditional())) {
-            elseGoto = elseGoto.getNextExpr();
-         }
          writeBlock(blockStart, elseGoto);
          write(" else ");
          writeBlock(elseGoto.getNextExpr(), null);
@@ -295,6 +329,7 @@ public void writeSequence(Instruction _first, Instruction _last) throws CodeGenE
          if (instruction instanceof CompositeInstruction) {
             writeComposite((CompositeInstruction) instruction);
          } else if (!instruction.getByteCode().equals(ByteCode.NONE)) {
+            writeHoistedExpressionLocalDeclarations(instruction);
             newLine();
             writeInstruction(instruction);
             write(";");
@@ -304,6 +339,130 @@ public void writeSequence(Instruction _first, Instruction _last) throws CodeGenE
 
    }
 
+   private Instruction getElseGoto(Instruction blockStart) {
+      Instruction elseGoto = blockStart;
+      while (!(elseGoto.isBranch() && elseGoto.asBranch().isUnconditional())) {
+         elseGoto = elseGoto.getNextExpr();
+      }
+      return (elseGoto);
+   }
+
+   private void writeHoistedLocalDeclarations(Instruction thenFirst, Instruction thenLast, Instruction elseFirst, Instruction elseLast) {
+      final LinkedHashMap<LocalVariableKey, LocalVariableInfo> thenDeclarations = collectBranchLocalDeclarations(thenFirst, thenLast);
+      final LinkedHashMap<LocalVariableKey, LocalVariableInfo> elseDeclarations = collectBranchLocalDeclarations(elseFirst, elseLast);
+
+      // Javac can split one source local into branch-local LVT entries; OpenCL needs one outer declaration.
+      for (Map.Entry<LocalVariableKey, LocalVariableInfo> entry : thenDeclarations.entrySet()) {
+         final LocalVariableKey key = entry.getKey();
+         if (elseDeclarations.containsKey(key) && !isLocalVariableDeclared(key)) {
+            final LocalVariableInfo localVariableInfo = entry.getValue();
+            currentLocalDeclarationScope().add(key);
+            newLine();
+            write(convertType(localVariableInfo.getVariableDescriptor(), true, false));
+            write(localVariableInfo.getVariableName());
+            write(" = 0;");
+         }
+      }
+   }
+
+   private void writeHoistedExpressionLocalDeclarations(Instruction instruction) {
+      final LinkedHashMap<LocalVariableKey, LocalVariableInfo> declarations = new LinkedHashMap<LocalVariableKey, LocalVariableInfo>();
+      collectNestedExpressionLocalDeclarations(instruction, declarations);
+      writeHoistedLocalDeclarations(declarations);
+   }
+
+   private void writeHoistedLocalDeclarations(LinkedHashMap<LocalVariableKey, LocalVariableInfo> declarations) {
+      for (Map.Entry<LocalVariableKey, LocalVariableInfo> entry : declarations.entrySet()) {
+         final LocalVariableKey key = entry.getKey();
+         if (!isLocalVariableDeclared(key)) {
+            final LocalVariableInfo localVariableInfo = entry.getValue();
+            currentLocalDeclarationScope().add(key);
+            newLine();
+            write(convertType(localVariableInfo.getVariableDescriptor(), true, false));
+            write(localVariableInfo.getVariableName());
+            write(" = 0;");
+         }
+      }
+   }
+
+   private void collectNestedExpressionLocalDeclarations(Instruction instruction,
+         LinkedHashMap<LocalVariableKey, LocalVariableInfo> declarations) {
+      if (instruction.getByteCode().equals(ByteCode.MULTI_ASSIGN) || instruction.getByteCode().equals(ByteCode.INLINE_ASSIGN)) {
+         collectLocalDeclarations(instruction, declarations);
+      }
+      for (Instruction child = instruction.getFirstChild(); child != null; child = child.getNextExpr()) {
+         collectLocalDeclarations(child, declarations);
+      }
+   }
+
+   private LinkedHashMap<LocalVariableKey, LocalVariableInfo> collectBranchLocalDeclarations(Instruction first, Instruction last) {
+      final LinkedHashMap<LocalVariableKey, LocalVariableInfo> declarations = new LinkedHashMap<LocalVariableKey, LocalVariableInfo>();
+      for (Instruction instruction = first; instruction != last; instruction = instruction.getNextExpr()) {
+         collectLocalDeclarations(instruction, declarations);
+      }
+      return (declarations);
+   }
+
+   private void collectLocalDeclarations(Instruction instruction, LinkedHashMap<LocalVariableKey, LocalVariableInfo> declarations) {
+      if (instruction instanceof AssignToLocalVariable) {
+         collectLocalDeclaration((AssignToLocalVariable) instruction, declarations);
+      }
+      if (instruction.getByteCode().equals(ByteCode.MULTI_ASSIGN)) {
+         final MultiAssignInstruction multiAssignInstruction = (MultiAssignInstruction) instruction;
+         for (Instruction from = multiAssignInstruction.getFrom(); from != multiAssignInstruction.getTo(); from = from.getNextExpr()) {
+            if (from instanceof AssignToLocalVariable) {
+               collectLocalDeclaration((AssignToLocalVariable) from, declarations);
+            }
+         }
+      } else if (instruction.getByteCode().equals(ByteCode.INLINE_ASSIGN)) {
+         final InlineAssignInstruction inlineAssignInstruction = (InlineAssignInstruction) instruction;
+         collectLocalDeclaration(inlineAssignInstruction.getAssignToLocalVariable(), declarations);
+      }
+      for (Instruction child = instruction.getFirstChild(); child != null; child = child.getNextExpr()) {
+         collectLocalDeclarations(child, declarations);
+      }
+   }
+
+   private void collectLocalDeclaration(AssignToLocalVariable assignToLocalVariable,
+         LinkedHashMap<LocalVariableKey, LocalVariableInfo> declarations) {
+      final LocalVariableInfo localVariableInfo = assignToLocalVariable.getLocalVariableInfo();
+      if (assignToLocalVariable.isDeclaration() && isHoistableLocalVariable(localVariableInfo)) {
+         declarations.put(new LocalVariableKey(localVariableInfo), localVariableInfo);
+      }
+   }
+
+   private boolean isHoistableLocalVariable(LocalVariableInfo localVariableInfo) {
+      if (localVariableInfo == null) {
+         return (false);
+      }
+      final String descriptor = localVariableInfo.getVariableDescriptor();
+      return (descriptor.length() == 1) && ("ZBCSIJFD".indexOf(descriptor.charAt(0)) >= 0);
+   }
+
+   private Set<LocalVariableKey> currentLocalDeclarationScope() {
+      if (localDeclarationScopes.empty()) {
+         localDeclarationScopes.push(new HashSet<LocalVariableKey>());
+      }
+      return (localDeclarationScopes.peek());
+   }
+
+   private boolean isLocalVariableDeclared(LocalVariableKey key) {
+      for (int i = localDeclarationScopes.size() - 1; i >= 0; i--) {
+         if (localDeclarationScopes.get(i).contains(key)) {
+            return (true);
+         }
+      }
+      return (false);
+   }
+
+   private boolean shouldDeclareLocalVariable(AssignToLocalVariable assignToLocalVariable) {
+      if (!assignToLocalVariable.isDeclaration()) {
+         return (false);
+      }
+      final LocalVariableInfo localVariableInfo = assignToLocalVariable.getLocalVariableInfo();
+      return (localVariableInfo == null) || !isLocalVariableDeclared(new LocalVariableKey(localVariableInfo));
+   }
+
    protected void writeGetterBlock(FieldEntry accessorVariableFieldEntry) {
       write("{");
       in();
@@ -320,7 +479,12 @@ protected void writeGetterBlock(FieldEntry accessorVariableFieldEntry) {
    public void writeBlock(Instruction _first, Instruction _last) throws CodeGenException {
       write("{");
       in();
-      writeSequence(_first, _last);
+      localDeclarationScopes.push(new HashSet<LocalVariableKey>());
+      try {
+         writeSequence(_first, _last);
+      } finally {
+         localDeclarationScopes.pop();
+      }
       out();
       newLine();
 
@@ -420,7 +584,7 @@ public void writeInstruction(Instruction _instruction) throws CodeGenException {
                  write(localVariableInfo.getVariableName());
              }
          } else {
-             if (assignToLocalVariable.isDeclaration()) {
+             if (shouldDeclareLocalVariable(assignToLocalVariable)) {
                  final String descriptor = localVariableInfo.getVariableDescriptor();
                  // Arrays always map to __global arrays
                  if (descriptor.startsWith("[")) {
@@ -682,7 +846,7 @@ public void writeInstruction(Instruction _instruction) throws CodeGenException {
          for (AssignToLocalVariable alv = stack.pop(); alv != null; alv = stack.size() > 0 ? stack.pop() : null) {
 
             final LocalVariableInfo localVariableInfo = alv.getLocalVariableInfo();
-            if (alv.isDeclaration()) {
+            if (shouldDeclareLocalVariable(alv)) {
                write(convertType(localVariableInfo.getVariableDescriptor(), true, false));
             }
             if (localVariableInfo == null) {
@@ -698,7 +862,7 @@ public void writeInstruction(Instruction _instruction) throws CodeGenException {
          final AssignToLocalVariable assignToLocalVariable = inlineAssignInstruction.getAssignToLocalVariable();
 
          final LocalVariableInfo localVariableInfo = assignToLocalVariable.getLocalVariableInfo();
-         if (assignToLocalVariable.isDeclaration()) {
+         if (shouldDeclareLocalVariable(assignToLocalVariable)) {
             // this is bad! we need a general way to hoist up a required declaration
             throw new CodeGenException("/* we can't declare this " + convertType(localVariableInfo.getVariableDescriptor(), true, false)
                   + " here */");

src/test/java/com/aparapi/codegen/test/AssignAndPassAsParameterSimple.java

@@ -26,4 +26,27 @@ public void run() {
         actuallyDoIt(z = 1);
     }
 }
-/**{Throws{CodeGenException}Throws}**/
+/**{OpenCL{
+ typedef struct This_s{
+ int passid;
+ }This;
+ int get_pass_id(This *this){
+ return this->passid;
+ }
+ void com_amd_aparapi_test_AssignAndPassAsParameterSimple__actuallyDoIt(This *this, int a){
+ return;
+ }
+ __kernel void run(
+ int passid
+ ){
+ This thisStruct;
+ This* this=&thisStruct;
+ this->passid = passid;
+ {
+ int z = 0;
+ com_amd_aparapi_test_AssignAndPassAsParameterSimple__actuallyDoIt(this, z=1);
+ return;
+ }
+ }
+
+ }OpenCL}**/

src/test/java/com/aparapi/codegen/test/AssignAndPassAsParameterSimpleTest.java

@@ -15,15 +15,36 @@
  */
 package com.aparapi.codegen.test;
 
-import com.aparapi.internal.exception.ClassParseException;
-import com.aparapi.internal.exception.CodeGenException;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class AssignAndPassAsParameterSimpleTest extends com.aparapi.codegen.CodeGenJUnitBase {
 
-    private static final String[] expectedOpenCL = null;
-    private static final Class<? extends com.aparapi.internal.exception.AparapiException> expectedException = CodeGenException.class;
+    private static final String[] expectedOpenCL = {
+        "typedef struct This_s{\n"
+        + " int passid;\n"
+        + " }This;\n"
+        + " int get_pass_id(This *this){\n"
+        + " return this->passid;\n"
+        + " }\n"
+        + " void com_aparapi_codegen_test_AssignAndPassAsParameterSimple__actuallyDoIt(This *this, int a){\n"
+        + " return;\n"
+        + " }\n"
+        + " __kernel void run(\n"
+        + " int passid\n"
+        + " ){\n"
+        + " This thisStruct;\n"
+        + " This* this=&thisStruct;\n"
+        + " this->passid = passid;\n"
+        + " {\n"
+        + " int z = 0;\n"
+        + " com_aparapi_codegen_test_AssignAndPassAsParameterSimple__actuallyDoIt(this, z=1);\n"
+        + " return;\n"
+        + " }\n"
+        + " }\n"
+        + "\n"
+        + " "
+    };
+    private static final Class<? extends com.aparapi.internal.exception.AparapiException> expectedException = null;
 
     @Test
     public void AssignAndPassAsParameterSimpleTest() {

src/test/java/com/aparapi/codegen/test/FirstAssignInExpression.java

@@ -39,7 +39,7 @@ int get_pass_id(This *this){
  return this->passid;
  }
 
- void func(This *this, int _arg){
+ void com_amd_aparapi_test_FirstAssignInExpression__func(This *this, int _arg){
  return;
  }
  __kernel void run(
@@ -49,8 +49,9 @@ __kernel void run(
  This* this=&thisStruct;
  this->passid = passid;
  {
- int result;
- func(this, result = 0);
+ int value = 1;
+ int result = 0;
+ com_amd_aparapi_test_FirstAssignInExpression__func(this, result=value);
  return;
  }
  }

src/test/java/com/aparapi/codegen/test/FirstAssignInExpression2Test.java

@@ -15,7 +15,6 @@
  */
 package com.aparapi.codegen.test;
 
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class FirstAssignInExpression2Test extends com.aparapi.codegen.CodeGenJUnitBase {
@@ -36,28 +35,27 @@ public class FirstAssignInExpression2Test extends com.aparapi.codegen.CodeGenJUn
         + " this->passid = passid;\n"
         + " {\n"
         + " int value = 1;\n"
-        + " int result=0;\n"
-        + " int assignMe=0;\n"
-        + " if (true){\n"
+        + " int result = 0;\n"
+        + " int assignMe = 0;\n"
+        + " if (value==value){\n"
         + " result = assignMe = value;\n"
         + " }else{\n"
         + " assignMe =1;\n"
         + " result=2;\n"
         + " }\n"
         + " result++;\n"
+        + " assignMe++;\n"
         + " return;\n"
         + " }\n"
         + " }\n"
         + " "};
     private static final Class<? extends com.aparapi.internal.exception.AparapiException> expectedException = null;
 
-    @Ignore
     @Test
     public void FirstAssignInExpression2Test() {
         test(com.aparapi.codegen.test.FirstAssignInExpression2.class, expectedException, expectedOpenCL);
     }
 
-    @Ignore
     @Test
     public void FirstAssignInExpression2TestWorksWithCaching() {
         test(com.aparapi.codegen.test.FirstAssignInExpression2.class, expectedException, expectedOpenCL);