You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by dj...@apache.org on 2006/02/04 00:12:19 UTC

svn commit: r374778 - in /db/derby/code/trunk/java: engine/org/apache/derby/iapi/services/classfile/ engine/org/apache/derby/impl/services/bytecode/ testing/org/apache/derbyTesting/functionTests/master/ testing/org/apache/derbyTesting/functionTests/tes...

Author: djd
Date: Fri Feb  3 15:12:08 2006
New Revision: 374778

URL: http://svn.apache.org/viewcvs?rev=374778&view=rev
Log:
DERBY-921 Support 32bit branch offsets for conditional logic in generated code.
Rewrite IF instructions to use a GOTO_W instruction when the branch offset is
greater than the 32767 allowed by the IF instructions. Replace GOTO instructions
with GOTO_W instruction when the branch offset is greater than the 32767.
Full details are in the Conditional.java file.

Write all instructions logically through CodeChunk methods containing the
code byte array knowledge within the CodeChunk class. Replaces the previous
direct modification of the code byte array by the Conditional class.

Increases number of logical operators to 800 for the test query in large code gen.

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/classfile/VMOpcode.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/BCClass.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/BCMethod.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/CodeChunk.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/Conditional.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/largeCodeGen.out
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/largeCodeGen.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/classfile/VMOpcode.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/classfile/VMOpcode.java?rev=374778&r1=374777&r2=374778&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/classfile/VMOpcode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/iapi/services/classfile/VMOpcode.java Fri Feb  3 15:12:08 2006
@@ -232,4 +232,11 @@
     short GOTO_W = 200;
     short JSR_W = 201;
     short BREAKPOINT = 202;
+    
+    
+    /**
+     * Maximum code length in a single method.
+     * See section 4.10 of JVM spec version 1.
+     */
+    int MAX_CODE_LENGTH = 65535;
 }

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/BCClass.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/BCClass.java?rev=374778&r1=374777&r2=374778&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/BCClass.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/BCClass.java Fri Feb  3 15:12:08 2006
@@ -371,7 +371,7 @@
 
 		ClassMember method = classHold.addMember(getter, sig, methodModifers);
 
-		CodeChunk chunk = new CodeChunk(true);
+		CodeChunk chunk = new CodeChunk();
 
 		// load 'this' if required
 		if (!staticField)
@@ -395,7 +395,7 @@
 		pda[0] = vmType;
 		sig = new BCMethodDescriptor(pda, VMDescriptor.VOID, factory).toString();
 		method = classHold.addMember(setter, sig, methodModifers);
-		chunk = new CodeChunk(true);
+		chunk = new CodeChunk();
 
 		// load 'this' if required
 		if (!staticField)

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/BCMethod.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/BCMethod.java?rev=374778&r1=374777&r2=374778&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/BCMethod.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/BCMethod.java Fri Feb  3 15:12:08 2006
@@ -141,7 +141,7 @@
 		myEntry = modClass.addMember(methodName, sig, modifiers);
 
 		// get code chunk
-		myCode = new CodeChunk(true);
+		myCode = new CodeChunk();
 	}
 	//
 	// MethodBuilder interface
@@ -180,7 +180,7 @@
 		// added in the sub method.
 		if (SanityManager.DEBUG)
 		{
-			if (myCode.getRelativePC() != 0)
+			if (myCode.getPC() != 0)
 				SanityManager.THROWASSERT("Adding exception after code generation " + exceptionClass
 						+ " to method " + getName());
 		}
@@ -1129,7 +1129,7 @@
 		if (condition != null)
 			return;
 		
-		int currentCodeSize = myCode.getRelativePC();
+		int currentCodeSize = myCode.getPC();
 		
 		// Overflow at >= 55,000 bytes which is someway
 		// below the limit of 65,535. Ideally overflow

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/CodeChunk.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/CodeChunk.java?rev=374778&r1=374777&r2=374778&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/CodeChunk.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/CodeChunk.java Fri Feb  3 15:12:08 2006
@@ -2,7 +2,7 @@
 
    Derby - Class org.apache.derby.impl.services.bytecode.CodeChunk
 
-   Copyright 1998, 2004 The Apache Software Foundation or its licensors, as applicable.
+   Copyright 1998, 2006 The Apache Software Foundation or its licensors, as applicable.
 
    Licensed under the Apache License, Version 2.0 (the "License");
    you may not use this file except in compliance with the License.
@@ -26,8 +26,10 @@
 
 import org.apache.derby.iapi.services.sanity.SanityManager;
 import org.apache.derby.iapi.services.classfile.VMOpcode;
+import org.apache.derby.iapi.services.io.ArrayOutputStream;
 
 import java.io.IOException;
+import java.util.Arrays;
 
 /**
  * This class represents a chunk of code in a CodeAttribute.
@@ -38,7 +40,13 @@
  * the need to generate a jump around each catch block, which
  * would be a forward reference.
  */
-class CodeChunk {
+final class CodeChunk {
+	
+	/**
+	 * Starting point of the byte code stream in the underlying stream/array.
+	 */
+	private static final int CODE_OFFSET = 8;
+		
 	// The use of ILOAD for the non-integer types is correct.
 	// We have to assume that the appropriate checks/conversions
 	// are defined on math operation results to ensure that
@@ -241,7 +249,13 @@
 		} catch (IOException ioe) {
 		}
 	}
-
+	void addInstrU4(short opcode, int operand) {
+		try {
+		cout.putU1(opcode);
+		cout.putU4(operand);
+		} catch (IOException ioe) {
+		}
+	}
 	void addInstrU1(short opcode, int operand) {
 		try {
 		cout.putU1(opcode);
@@ -309,42 +323,55 @@
 		}
 	}
 
-	/** Add an arbitrary array of bytes */
-	void addChunk(CodeChunk other) {
-		
+	/** Get the current program counter */
+	public int getPC() {
+		return cout.size() + pcDelta;
+	}
+	
+	/**
+	 * The delta between cout.size() and the pc.
+	 * For an initial code chunk this is -8 (CODE_OFFSET)
+	 * since 8 bytes are written.
+	 * For a nested CodeChunk return by insertCodeSpace the delta
+	 * corresponds to the original starting pc.
+	 * @see insetCodeSpace
+	 */
+	private final int pcDelta;
+
+	CodeChunk() {
+        cout = new ClassFormatOutput();
 		try {
-			other.getCout().writeTo(cout);
+			cout.putU2(0); // max_stack, placeholder for now
+			cout.putU2(0); // max_locals, placeholder for now
+			cout.putU4(0); // code_length, placeholder 4 now
 		} catch (IOException ioe) {
 		}
+		pcDelta = - CodeChunk.CODE_OFFSET;
 	}
-
-	/** Get the ClassFormatOutput */
-	ClassFormatOutput getCout() {
-		return cout;
-	}
-
-	/** Get the current PC */
-	public int getRelativePC() {
-		return cout.size() - codeOffset;
-	}
-
-	CodeChunk(boolean main) {
-        cout = new ClassFormatOutput();
-		if (main) {
-			try {
-				cout.putU2(0); // max_stack, placeholder for now
-				cout.putU2(0); // max_locals, placeholder for now
-				cout.putU4(0); // code_length, placeholder 4 now
-			} catch (IOException ioe) {
-			}
-			
-			codeOffset = 8; // just wrote eight bytes
+	
+	/**
+	 * Return a CodeChunk that has limited visibility into
+	 * this CodeChunk. Used when a caller needs to insert instructions
+	 * into an existing stream.
+	 * @param pc
+	 * @param byteCount
+	 * @throws IOException 
+	 */
+	private CodeChunk(CodeChunk main, int pc, int byteCount)
+	{
+		ArrayOutputStream aos =
+			new ArrayOutputStream(main.cout.getData());
+		
+		try {
+			aos.setPosition(CODE_OFFSET + pc);
+			aos.setLimit(byteCount);
+		} catch (IOException e) {
 		}
+		
+		cout = new ClassFormatOutput(aos);
+		pcDelta = pc;
 	}
 
-	/* This is the PC relative to the beginning of this code chunk */
-	private int codeOffset;
-
 	private final ClassFormatOutput cout;
 
 	/**
@@ -374,8 +401,9 @@
 		codeBytes[3] = (byte)(maxLocals );
 
 		// code_length is in bytes 4-7
-		if (mb != null && codeLength > 65536)
-			mb.cb.addLimitExceeded(mb, "code_length", 65536, codeLength);
+		if (mb != null && codeLength > VMOpcode.MAX_CODE_LENGTH)
+			mb.cb.addLimitExceeded(mb, "code_length",
+					VMOpcode.MAX_CODE_LENGTH, codeLength);
 		codeBytes[4] = (byte)(codeLength >> 24 );
 		codeBytes[5] = (byte)(codeLength >> 16 );
 		codeBytes[6] = (byte)(codeLength >> 8 );
@@ -390,7 +418,7 @@
 	void complete(BCMethod mb, ClassHolder ch,
 			ClassMember method, int maxStack, int maxLocals) {
 
-		int codeLength = getRelativePC();
+		int codeLength = getPC();
 
 		ClassFormatOutput out = cout;
 
@@ -427,5 +455,94 @@
 		fixLengths(mb, maxStack, maxLocals, codeLength);
 		method.addAttribute("Code", out);
 	}
+	
+	/**
+	 * Return the opcode at the given pc.
+	 */
+	short getOpcode(int pc)
+	{
+		return (short) (cout.getData()[CODE_OFFSET + pc] & 0xff);
+	}
+	
+	/**
+	 * Insert room for byteCount bytes after the instruction at pc
+	 * and prepare to replace the instruction at pc. The instruction
+	 * at pc is not modified by this call, space is allocated after it.
+	 * The newly inserted space will be filled with NOP instructions.
+	 * 
+	 * Returns a CodeChunk positioned at pc and available to write
+	 * instructions upto (byteCode + length(existing instruction at pc) bytes.
+	 * 
+	 * This chunk is left correctly positioned at the end of the code
+	 * stream, ready to accept more code. Its pc will have increased by
+	 * additionalBytes.
+	 * 
+	 * It is the responsibility of the caller to patch up any
+	 * branches or gotos.
+	 * 
+	 * @param pc
+	 * @param additionalBytes
+	 * @return
+	 */
+	CodeChunk insertCodeSpace(int pc, int additionalBytes)
+	{
+		short existingOpcode = getOpcode(pc);
+		int lengthOfExistingInstruction;
+		if (existingOpcode == VMOpcode.GOTO_W)
+			lengthOfExistingInstruction = 5;
+		else
+			lengthOfExistingInstruction = 3;
+		
+		
+		if (additionalBytes > 0)
+		{
+			// Size of the current code after this pc.
+			int sizeToMove = (getPC() - pc) - lengthOfExistingInstruction;
 
+			// Increase the code by the number of bytes to be
+			// inserted. These NOPs will be overwritten by the
+			// moved code by the System.arraycopy below.
+			// It's assumed that the number of inserted bytes
+			// is small, one or two instructions worth, so it
+			// won't be a performance issue.
+			for (int i = 0; i < additionalBytes; i++)
+				addInstr(VMOpcode.NOP);
+		
+			// Must get codeBytes here as the array might have re-sized.
+			byte[] codeBytes = cout.getData();
+			
+			int byteOffset = CODE_OFFSET + pc + lengthOfExistingInstruction;
+					
+			
+			// Shift the existing code stream down
+			// pc + 3
+			// Pc + 3 + 5
+			System.arraycopy(
+					codeBytes, byteOffset,
+					codeBytes, byteOffset + additionalBytes,
+					sizeToMove);
+			
+			// Place NOPs in the space just freed by the move.
+			// This is not required, it ias assumed the caller
+			// will overwrite all the bytes they requested, but
+			// to be safe fill in with NOPs rather than leaving code
+			// that could break the verifier.
+			Arrays.fill(codeBytes, byteOffset, byteOffset + additionalBytes,
+					(byte) VMOpcode.NOP);
+		}
+		
+		// The caller must overwrite the original instruction
+		// at pc, thus increase the range of the limit stream
+		// created to include those bytes.
+		additionalBytes += lengthOfExistingInstruction;
+		
+		// Now the caller needs to fill in the instructions
+		// that make up the modified byteCount bytes of bytecode stream.
+		// Return a CodeChunk that can be used for this and
+		// is limited to only those bytes.
+		// The pc of the original code chunk is left unchanged.
+		
+		return new CodeChunk(this, pc, additionalBytes);
+						
+	}
 }

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/Conditional.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/Conditional.java?rev=374778&r1=374777&r2=374778&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/Conditional.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/services/bytecode/Conditional.java Fri Feb  3 15:12:08 2006
@@ -2,7 +2,7 @@
 
    Derby - Class org.apache.derby.impl.services.bytecode.Conditional
 
-   Copyright 2000, 2004 The Apache Software Foundation or its licensors, as applicable.
+   Copyright 2000, 2006 The Apache Software Foundation or its licensors, as applicable.
 
    Licensed under the Apache License, Version 2.0 (the "License");
    you may not use this file except in compliance with the License.
@@ -23,32 +23,121 @@
 import org.apache.derby.iapi.services.sanity.SanityManager;
 
 /**
-	A code chunk that gets pushed to handle if-else blocks.
-	When this is created the mainChunk will already have
-	the conditional check code.
-
+	A Conditional represents an if/then/else block.
+	When this is created the code  will already have
+	the conditional check code. The code is optimized for branch
+	offsets that fit in 2 bytes, though will handle 4 byte offsets.
+<code>
      if condition
 	 then code
 	 else code
-
+</code>
      what actually gets built is
+<code>
+     if !condition branch to eb:
+	  then code
+	  goto end:  // skip else
+	 eb:
+	  else code
+	 end:
+</code>
 
-     if !condition goto eb:
+    If no else condition was provided then the code is:
+    
+<code>
+     if !condition branch to end:
 	  then code
-	  goto end:
+	 end:
+</code>
+
+Note all branches here are using relative offsets, not absolute program counters.
+
+If the then code leads to the conditional branch offset being too big (>32k)
+because the then code is larger than 32767 bytes then this is built:
+<code>
+     // when else code is present
+     if condition branch to tb: (relative offset +8)
+     goto_w eb: // indirect for else block (5 bytes)
+     tb:
+	    then code (> 32767 bytes)
+	    goto end:
 	 eb:
 	  else code
 	 end:
+</code>
+
+<code>
+     // when only then code is present
+     if condition branch to tb: (relative offset +8)
+     goto_w end: // indirect for else block (5 bytes)
+     tb:
+	    then code (> 32767 bytes)
+	 end:
+</code>
+
+If there is an else branch and only it is larger than 32767 bytes then
+the code is:
+
+<code>
+     if !condition branch to eb: (offset increased by two over previous value)
+	  then code
+	  goto_w end:  // skip else
+	 eb:
+	  else code (> 32767 bytes)
+	 end:
+</code>
+
+This has one special case where the size of conditional branch to eb:
+now must change from a 16bit value to a 32 bit value. The generated code
+for this is the same as when both the then code and the else code require
+32bit offsets for the branches. This code is:
+
+<code>
+     if condition branch to tb: (relative offset +8)
+     goto_w eb: // indirect for else block (5 bytes)
+     tb:
+	    then code (> 32767 bytes)
+	    goto_w end:
+	 eb:
+	  else code (> 32767 bytes)
+	 end:
+</code>
+
+In theory, at the moment this should not happen as this would mean a total
+code size that exceeds the limit on the code size for a method (64k). This
+code handles this case as it does occur if the limit for a branch is lowered
+for testing purposes, to ensure the complete set of branch re-write code works.
+This lowering of the limit can be done by changing the constant BRANCH16LIMIT.
+  
 */
 class Conditional {
+	
+	/**
+	 * Limit of a 16 bit branch.
+	 * <P>
+	 * If broad testing of the switch from 16bit to 32bit
+	 * offsets is required then this constant can be reduced
+	 * to a lower value, say 50 and run complete tests. This
+	 * will cover all the combinations. This works because the
+	 * GOTO_W instruction works with any offset value.
+	 */
+	private static final int BRANCH16LIMIT = 32767;
+
 
 	private final Conditional parent;
 	/**
-	 * Offset in the code stream of the 'if' opcode.
+	 * pc of the 'if' opcode.
 	 */
-	private final int   ifOffset;
+	private final int   if_pc;
+	
 	private Type[]	stack;
-	private int thenGotoOffset;
+	
+	/**
+	 * pc of the GOTO added at the end of the then block
+	 * to transfer control to the end of this conditional.
+	 * That is at the end of the else block.
+	 */
+	private int thenGoto_pc;
 
 	/**
 	 * Start a conditional block.
@@ -59,9 +148,9 @@
 	 */
 	Conditional(Conditional parent, CodeChunk chunk, short ifOpcode, Type[] entryStack) {
 		this.parent = parent;
-		ifOffset = chunk.getRelativePC();
+		if_pc = chunk.getPC();
 		this.stack = entryStack;
-
+		
 		// reserve the space for the branch, will overwrite later
 		// with the correct branch offset.
 		chunk.addInstrU2(ifOpcode, 0);
@@ -74,14 +163,19 @@
 	 * @return the type stack on entering the then block
 	 */
 	Type[] startElse(BCMethod mb, CodeChunk chunk, Type[] thenStack) {
-
-		thenGotoOffset = chunk.getRelativePC();
-
-		// reserve space for the goto we will be adding
+		
+		// reserve space for the goto end we will be adding
 		chunk.addInstrU2(VMOpcode.GOTO, 0);
 
-		// fill in the branch opcode
-		fillIn(mb, chunk, ifOffset);
+		// fill in the branch opcode to branch to
+		// the code after the goto, which is the current pc.
+		fillIn(mb, chunk, if_pc, chunk.getPC());
+		
+		// Cannot use the pc before adding the GOTO above
+		// as the fillIn may insert bytes that move the GOTO,
+		// thus calculate at the end, and subtract the number of
+		// instructions in a goto to get its pc.
+		thenGoto_pc = chunk.getPC() - 3;
 		
 		Type[] entryStack = stack;
 		stack = thenStack;
@@ -98,14 +192,17 @@
 	 * @return The conditional this conditional was nested in, if any.
 	 */
 	Conditional end(BCMethod mb, CodeChunk chunk, Type[] elseStack, int stackNumber) {
-
-		if (thenGotoOffset == 0) {
-			// no else condition
-			fillIn(mb, chunk, ifOffset);
+		int branch_pc;
+		if (thenGoto_pc == 0) {
+			// no else condition, make the conditional branch to the end
+			branch_pc = if_pc;
 		} else {
-			fillIn(mb, chunk, thenGotoOffset);
+			// otherwise make the goto branch to the end
+			branch_pc = thenGoto_pc;
 		}
 		
+		fillIn(mb, chunk, branch_pc, chunk.getPC());
+		
 		if (SanityManager.DEBUG)
 		{
 			if (stackNumber != stack.length)
@@ -125,27 +222,181 @@
 	}
 
 	/**
+	 * Fill in the offsets for a conditional or goto instruction that
+	 * were dummied up as zero during code generation. Handles modifying
+	 * branch logic when the offset for the branch is greater than can
+	 * fit in 16 bits. In this case a GOTO_W with a 32 bit offset will
+	 * be used, see details within the method for how this is acheived
+	 * in all situations. This method might insert instructions in the
+	 * already generated byte code, thus increasing the program counter.
 	 * 
+	 * @param mb Method this conditional is for
 	 * @param chunk Our code chunk
-	 * @param whereFrom Offset of the branch opcode in the code stream
+	 * @param branch_pc pc of the branch or goto opcode in the code stream
+	 * @param target_pc pc where we want to jump to.
 	 */
-	private void fillIn(BCMethod mb, CodeChunk chunk, int whereFrom) {
+	private void fillIn(BCMethod mb, CodeChunk chunk,
+			int branch_pc, int target_pc) {
 
-		byte[] codeBytes = chunk.getCout().getData();
+		int offset = target_pc - branch_pc;
 
-		int offset = chunk.getRelativePC() - whereFrom;
+		// Following code assumes that this class only
+		// generates forward jumps. Jump of zero is
+		// wrong as well, would be infinite loop or stack problems.
+		if (SanityManager.DEBUG)
+		{
+			if (offset <= 0)
+				SanityManager.THROWASSERT("Conditional branch zero or negative " + offset);
+		}
+
+		// Original opcode written.
+		short branchOpcode = chunk.getOpcode(branch_pc);
 		
-		// branch offsets are a 16bit signed value, this implementation
-		// currently only generates forward branches
-		if (offset > 32767)
-			mb.cb.addLimitExceeded(mb, "branch_offset", 32767, offset);
+		// Handle 16bit offsets, two byte.
+		if (offset <= BRANCH16LIMIT)
+		{
+			// Code was already setup for two byte offsets,
+			// branch or goto instruction was written with
+			// offset zero, ready to be overwritten by this code.
+			CodeChunk mod = chunk.insertCodeSpace(branch_pc, 0);
+			mod.addInstrU2(branchOpcode, offset);
+			return;
+		}
+		
+		if (branchOpcode == VMOpcode.GOTO)
+		{
+			// Ensure the pc we are jumping to (the current pc)
+			// is within bounds of a valid method *after*
+			// we have added the extra bytes.
+			if ((target_pc + 2) >= VMOpcode.MAX_CODE_LENGTH)
+			{
+				mb.cb.addLimitExceeded(mb, "goto_target",
+						VMOpcode.MAX_CODE_LENGTH, target_pc + 2);
+				
+				// even if we fail continue to generate the correct code
+				// so that the assumptions in the patch up code are not broken.
+			}
+			
+		
+			// Change the GOTO to a GOTO_W, which means
+			// inserting 2 bytes into the stream.
+			CodeChunk mod = chunk.insertCodeSpace(branch_pc, 2);
+			
+			// Offset we are jumping to is now two bytes futher away
+			offset += 2;
+			
+			// replace the original GOTO with a GOTO_W
+			mod.addInstrU4(VMOpcode.GOTO_W, offset);
+			
+			// Now need to patch up the original conditional
+			// as the else code it was branching to is now
+			// another two bytes away.
+			// There are three cases, given the original branch_offset:
+			//
+			// 1) branch_offset 16bit, branch_offset+2 16 bit
+			// 2) branch_offset 16bit, branch_offset+2 32 bit
+			// 3) branch_offset 32bit, branch_offset+2 32 bit
+			//
+			int startElse_pc = mod.getPC();
+			
+			int branchOffset = startElse_pc - if_pc;
+							
+			if (branchOffset <= BRANCH16LIMIT + 2)
+			{					
+				// case 1) branch_offset 16bit, branch_offset+2 16 bit
+                // case 2) branch_offset 16bit, branch_offset+2 32 bit
+				//
+				// Branch to the else code is on the original conditional
+				
+				// both handled by the standard fillIn method.
+				fillIn(mb, chunk, if_pc, mod.getPC());
+				return;
 
-		// Skip the eight byte header at the start of the
-		// byte array, the eight bytes are the CodeAttribute header.
-		whereFrom += 8;
+			}
+
+			// branch to the else code was changed from the conditional
+			// to a GOTO_W as the branch was out of the range of the
+			// conditional.
+			
+			// Overwrite the offset of the existing GOTO_W, the instruction
+			// after the conditional instruction, which is three bytes long
+			mod = chunk.insertCodeSpace(if_pc + 3, 0);
+			
+			// Above branchOffset was calculated from the conditional
+			// but we need to branch from the GOTO_W that was inserted
+			// which is three bytes after the conditional.
+			branchOffset -= 3;
+			
+			mod.addInstrU4(VMOpcode.GOTO_W, branchOffset);
+			return;
+
+		}
+		else
+		{
+			// Ensure the pc we are jumping to (the current pc)
+			// is within bounds of a valid method *after*
+			// we have added the extra bytes.
+			if ((target_pc + 5) >= VMOpcode.MAX_CODE_LENGTH)
+			{
+			    mb.cb.addLimitExceeded(mb,
+			    		"branch_target", VMOpcode.MAX_CODE_LENGTH, target_pc + 5);
+				// even if we fail continue to generate the correct code
+				// so that the assumptions in the patch up code are not broken.
+			}
+
+			// Conditional branch
+			// branch on the conditional, need to add
+			// indirection. Basically changing
+			// (actual conditional might be different)
+			// Note branch inverting.
+			//
+			// IFNONNULL branch offset (to else code)
+			//   <then code>
+			// GOTO end:
+			//   <else code>
+			// end:
+			// to
+			//
+			// IFNULL branch +8  (to then code, 3 bytes in stream)
+			// GOTO_W offset* (to else code, 5 new bytes in stream)
+			//    <then code>
+			// GOTO end:
+			//    <else code>
+					
+			// Invert branch.
+			switch (branchOpcode)
+			{
+			case VMOpcode.IFNONNULL:
+				branchOpcode = VMOpcode.IFNULL;
+				break;
+			case VMOpcode.IFEQ:
+				branchOpcode = VMOpcode.IFNE;
+				break;
+			default:
+				if (SanityManager.DEBUG)
+					SanityManager.THROWASSERT("Conditional does not handle opcode " + branchOpcode);
 				
-		codeBytes[whereFrom + 1] = (byte)(offset >> 8 );
-		codeBytes[whereFrom + 2] = (byte)(offset);
+			}
+												
+			// Thus we need to insert 5 bytes
+			//
+			CodeChunk mod = chunk.insertCodeSpace(branch_pc, 5);
+			
+			// mod is positioned at the current branch.
+			mod.addInstrU2(branchOpcode, 8);
+											
+			// Indirect goto for the conditional else block or end.
+			// Offset was from the comparision instruction to the
+			// start of the real code. Now the branch location
+			// is an additional two bytes away, because this
+			// GOTO_W instruction occupies 5 bytes, and the original
+			// branch 3.
+			offset += 2;
+			
+			mod.addInstrU4(VMOpcode.GOTO_W, offset);
+	
+			return;	
+		}
 	}
 
 

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/largeCodeGen.out
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/largeCodeGen.out?rev=374778&r1=374777&r2=374778&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/largeCodeGen.out (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/largeCodeGen.out Fri Feb  3 15:12:08 2006
@@ -1,8 +1,21 @@
+PASS: PREPARE: Logical operators with 200 parameters
 PASS: Logical operators with 200 parameters
+PASS: PREPARE: Logical operators with 300 parameters
 PASS: Logical operators with 300 parameters
+PASS: PREPARE: Logical operators with 400 parameters
 PASS: Logical operators with 400 parameters
-FAILED QUERY: Logical operators with 500 parameters.
+PASS: PREPARE: Logical operators with 500 parameters
+PASS: Logical operators with 500 parameters
+PASS: PREPARE: Logical operators with 600 parameters
+PASS: Logical operators with 600 parameters
+PASS: PREPARE: Logical operators with 700 parameters
+PASS: Logical operators with 700 parameters
+PASS: PREPARE: Logical operators with 800 parameters
+PASS: Logical operators with 800 parameters
+FAILED QUERY: Logical operators with 900 parameters.
+PASS: PREPARE: IN clause with 3300 parameters
 PASS: IN clause with 3300 parameters
+PASS: PREPARE: IN clause with 3400 parameters
 PASS: IN clause with 3400 parameters
 FAILED QUERY: IN clause with 3500 parameters.
 PASS: PREPARE: SELECT with 800 unions

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/largeCodeGen.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/largeCodeGen.java?rev=374778&r1=374777&r2=374778&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/largeCodeGen.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/largeCodeGen.java Fri Feb  3 15:12:08 2006
@@ -70,7 +70,7 @@
 		PreparedStatement ps;
 		try {
 			ps = con.prepareStatement(sqlBuffer.toString());
-			
+			System.out.println("PASS: PREPARE: " + testName);
 			if (TEST_QUERY_EXECUTION)
 			{
 				for (int i = 1; i <= numParams; i++)
@@ -101,6 +101,8 @@
 	private static void testLogicalOperators(Connection con)  throws SQLException {
 		 
 		// svn 372388 trunk - passed @ 400
+		// Fix to DERBY-921 - passed @ 800
+		//   DERBY-921 - support 32bit branch offsets
 		 for (int count = 200; count <= 10000 ; count += 100)
 		 {
 			 // keep testing until it fails with linkage error



Re: svn commit: r374778 - in /db/derby/code/trunk/java: engine/org/apache/derby/iapi/services/classfile/ engine/org/apache/derby/impl/services/bytecode/ testing/org/apache/derbyTesting/functionTests/master/ testing/org/apache/derbyTesting/functionTests/tes...

Posted by Daniel John Debrunner <dj...@apache.org>.
Kathey Marsden wrote:

> djd@apache.org wrote:
> Thanks Dan for the continued progress on my favorite issue.   I  am
> looking forward to studying the patch sometime to actually understand
> it, but in the meanwhile I just looked at the little stuff.
> 
> 1)  On the javadoc I found that eclipse  generates javadoc like this,
> but Rick reported that it actually causes a warning  because there is
> nothing after the @return
> 
> 	 * @param pc
> +	 * @param additionalBytes
> +	 * @return
> +	 */

Darn, thought I caught all of those. Sorry, now I already have some
changes in that file to address the more generic code overflow issues.

> 2) Would it make sense to bump up the starting value of the loop in the
> test,  now that you have made so much progress?

The first few queries are pretty quick it's the later big union queries
that take the time. I'm working on further changes in this area, once I
touch the test again I will look at making sure the starting points are
not too far away from the failing points.

Dan.


Re: svn commit: r374778 - in /db/derby/code/trunk/java: engine/org/apache/derby/iapi/services/classfile/ engine/org/apache/derby/impl/services/bytecode/ testing/org/apache/derbyTesting/functionTests/master/ testing/org/apache/derbyTesting/functionTests/tes...

Posted by Kathey Marsden <km...@sbcglobal.net>.
djd@apache.org wrote:
Thanks Dan for the continued progress on my favorite issue.   I  am
looking forward to studying the patch sometime to actually understand
it, but in the meanwhile I just looked at the little stuff.

1)  On the javadoc I found that eclipse  generates javadoc like this,
but Rick reported that it actually causes a warning  because there is
nothing after the @return

	 * @param pc
+	 * @param additionalBytes
+	 * @return
+	 */



2) Would it make sense to bump up the starting value of the loop in the
test,  now that you have made so much progress?

+PASS: PREPARE: Logical operators with 200 parameters
 PASS: Logical operators with 200 parameters
+PASS: PREPARE: Logical operators with 300 parameters
 PASS: Logical operators with 300 parameters
+PASS: PREPARE: Logical operators with 400 parameters
 PASS: Logical operators with 400 parameters
-FAILED QUERY: Logical operators with 500 parameters.
+PASS: PREPARE: Logical operators with 500 parameters
+PASS: Logical operators with 500 parameters
+PASS: PREPARE: Logical operators with 600 parameters
+PASS: Logical operators with 600 parameters
+PASS: PREPARE: Logical operators with 700 parameters
+PASS: Logical operators with 700 parameters
+PASS: PREPARE: Logical operators with 800 parameters
+PASS: Logical operators with 800 parameters
+FAILED QUERY: Logical operators with 900 parameters.