mirror of
https://github.com/openjdk/jdk.git
synced 2025-09-20 11:04:34 +02:00
8009170: Regression: javac generates redundant bytecode in assignop involving arrays
Reviewed-by: mcimadamore
This commit is contained in:
Vicente Romero
parent
7fe4d602a6
commit
b04bec8156
3 changed files with 159 additions and 34 deletions
|
|
@ -3158,38 +3158,59 @@ public class Lower extends TreeTranslator {
|
|||
}
|
||||
|
||||
public void visitAssignop(final JCAssignOp tree) {
|
||||
JCTree lhsAccess = access(TreeInfo.skipParens(tree.lhs));
|
||||
final boolean boxingReq = !tree.lhs.type.isPrimitive() &&
|
||||
tree.operator.type.getReturnType().isPrimitive();
|
||||
|
||||
// boxing required; need to rewrite as x = (unbox typeof x)(x op y);
|
||||
// or if x == (typeof x)z then z = (unbox typeof x)((typeof x)z op y)
|
||||
// (but without recomputing x)
|
||||
JCTree newTree = abstractLval(tree.lhs, new TreeBuilder() {
|
||||
public JCTree build(final JCTree lhs) {
|
||||
JCTree.Tag newTag = tree.getTag().noAssignOp();
|
||||
// Erasure (TransTypes) can change the type of
|
||||
// tree.lhs. However, we can still get the
|
||||
// unerased type of tree.lhs as it is stored
|
||||
// in tree.type in Attr.
|
||||
Symbol newOperator = rs.resolveBinaryOperator(tree.pos(),
|
||||
newTag,
|
||||
attrEnv,
|
||||
tree.type,
|
||||
tree.rhs.type);
|
||||
JCExpression expr = (JCExpression)lhs;
|
||||
if (expr.type != tree.type)
|
||||
expr = make.TypeCast(tree.type, expr);
|
||||
JCBinary opResult = make.Binary(newTag, expr, tree.rhs);
|
||||
opResult.operator = newOperator;
|
||||
opResult.type = newOperator.type.getReturnType();
|
||||
JCExpression newRhs = boxingReq ?
|
||||
make.TypeCast(types.unboxedType(tree.type),
|
||||
opResult) :
|
||||
if (boxingReq || lhsAccess.hasTag(APPLY)) {
|
||||
// boxing required; need to rewrite as x = (unbox typeof x)(x op y);
|
||||
// or if x == (typeof x)z then z = (unbox typeof x)((typeof x)z op y)
|
||||
// (but without recomputing x)
|
||||
JCTree newTree = abstractLval(tree.lhs, new TreeBuilder() {
|
||||
public JCTree build(final JCTree lhs) {
|
||||
JCTree.Tag newTag = tree.getTag().noAssignOp();
|
||||
// Erasure (TransTypes) can change the type of
|
||||
// tree.lhs. However, we can still get the
|
||||
// unerased type of tree.lhs as it is stored
|
||||
// in tree.type in Attr.
|
||||
Symbol newOperator = rs.resolveBinaryOperator(tree.pos(),
|
||||
newTag,
|
||||
attrEnv,
|
||||
tree.type,
|
||||
tree.rhs.type);
|
||||
JCExpression expr = (JCExpression)lhs;
|
||||
if (expr.type != tree.type)
|
||||
expr = make.TypeCast(tree.type, expr);
|
||||
JCBinary opResult = make.Binary(newTag, expr, tree.rhs);
|
||||
opResult.operator = newOperator;
|
||||
opResult.type = newOperator.type.getReturnType();
|
||||
JCExpression newRhs = boxingReq ?
|
||||
make.TypeCast(types.unboxedType(tree.type), opResult) :
|
||||
opResult;
|
||||
return make.Assign((JCExpression)lhs, newRhs).setType(tree.type);
|
||||
}
|
||||
});
|
||||
result = translate(newTree);
|
||||
return make.Assign((JCExpression)lhs, newRhs).setType(tree.type);
|
||||
}
|
||||
});
|
||||
result = translate(newTree);
|
||||
return;
|
||||
}
|
||||
tree.lhs = translate(tree.lhs, tree);
|
||||
tree.rhs = translate(tree.rhs, tree.operator.type.getParameterTypes().tail.head);
|
||||
|
||||
// If translated left hand side is an Apply, we are
|
||||
// seeing an access method invocation. In this case, append
|
||||
// right hand side as last argument of the access method.
|
||||
if (tree.lhs.hasTag(APPLY)) {
|
||||
JCMethodInvocation app = (JCMethodInvocation)tree.lhs;
|
||||
// if operation is a += on strings,
|
||||
// make sure to convert argument to string
|
||||
JCExpression rhs = (((OperatorSymbol)tree.operator).opcode == string_add)
|
||||
? makeString(tree.rhs)
|
||||
: tree.rhs;
|
||||
app.args = List.of(rhs).prependList(app.args);
|
||||
result = app;
|
||||
} else {
|
||||
result = tree;
|
||||
}
|
||||
}
|
||||
|
||||
/** Lower a tree of the form e++ or e-- where e is an object type */
|
||||
|
|
|
|||
|
|
@ -34,27 +34,60 @@ public class DiffResultAfterSameOperationInnerClasses {
|
|||
private int j = 1;
|
||||
public String s1 = "Hi, ";
|
||||
private String s2 = "Hi, ";
|
||||
public int arr1[] = new int[]{1};
|
||||
public int arr2[] = new int[]{1};
|
||||
|
||||
public static void main(String[] args) {
|
||||
InnerClass inner =
|
||||
new DiffResultAfterSameOperationInnerClasses().new InnerClass();
|
||||
if (!inner.test()) {
|
||||
DiffResultAfterSameOperationInnerClasses theTest =
|
||||
new DiffResultAfterSameOperationInnerClasses();
|
||||
InnerClass inner = theTest.new InnerClass();
|
||||
if (!inner.test1()) {
|
||||
throw new AssertionError("Different results after same calculation");
|
||||
}
|
||||
|
||||
theTest.resetVars();
|
||||
if (!inner.test2()) {
|
||||
throw new AssertionError("Different results after same calculation");
|
||||
}
|
||||
}
|
||||
|
||||
void resetVars() {
|
||||
i = 1;
|
||||
j = 1;
|
||||
s1 = "Hi, ";
|
||||
s2 = "Hi, ";
|
||||
arr1[0] = 1;
|
||||
arr2[0] = 1;
|
||||
}
|
||||
|
||||
class InnerClass {
|
||||
public boolean test() {
|
||||
public boolean test1() {
|
||||
i += i += 1;
|
||||
j += j += 1;
|
||||
|
||||
arr1[0] += arr1[0] += 1;
|
||||
arr2[0] += arr2[0] += 1;
|
||||
|
||||
s1 += s1 += "dude";
|
||||
s2 += s2 += "dude";
|
||||
|
||||
System.out.println("s1 = " + s1);
|
||||
System.out.println("s2 = " + s2);
|
||||
return (i == j && i == 3 &&
|
||||
arr1[0] == arr2[0] && arr2[0] == 3 &&
|
||||
s1.equals(s2) && s1.endsWith("Hi, Hi, dude"));
|
||||
}
|
||||
|
||||
public boolean test2() {
|
||||
(i) += (i) += 1;
|
||||
(j) += (j) += 1;
|
||||
|
||||
(arr1[0])+= (arr1[0]) += 1;
|
||||
(arr2[0])+= (arr2[0]) += 1;
|
||||
|
||||
(s1) += (s1) += "dude";
|
||||
(s2) += (s2) += "dude";
|
||||
|
||||
return (i == j && i == 3 &&
|
||||
arr1[0] == arr2[0] && arr2[0] == 3 &&
|
||||
s1.equals(s2) && s1.endsWith("Hi, Hi, dude"));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,71 @@
|
|||
/*
|
||||
* Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
|
||||
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
|
||||
*
|
||||
* This code is free software; you can redistribute it and/or modify it
|
||||
* under the terms of the GNU General Public License version 2 only, as
|
||||
* published by the Free Software Foundation.
|
||||
*
|
||||
* This code is distributed in the hope that it will be useful, but WITHOUT
|
||||
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
|
||||
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
|
||||
* version 2 for more details (a copy is included in the LICENSE file that
|
||||
* accompanied this code).
|
||||
*
|
||||
* You should have received a copy of the GNU General Public License version
|
||||
* 2 along with this work; if not, write to the Free Software Foundation,
|
||||
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
|
||||
*
|
||||
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
|
||||
* or visit www.oracle.com if you need additional information or have any
|
||||
* questions.
|
||||
*/
|
||||
|
||||
/*
|
||||
* @test
|
||||
* @bug 8009170
|
||||
* @summary Regression: javac generates redundant bytecode in assignop involving
|
||||
* arrays
|
||||
* @run main RedundantByteCodeInArrayTest
|
||||
*/
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
|
||||
import com.sun.tools.classfile.Attribute;
|
||||
import com.sun.tools.classfile.ClassFile;
|
||||
import com.sun.tools.classfile.Code_attribute;
|
||||
import com.sun.tools.classfile.Code_attribute.InvalidIndex;
|
||||
import com.sun.tools.classfile.ConstantPool;
|
||||
import com.sun.tools.classfile.ConstantPoolException;
|
||||
import com.sun.tools.classfile.Descriptor.InvalidDescriptor;
|
||||
import com.sun.tools.classfile.Method;
|
||||
|
||||
public class RedundantByteCodeInArrayTest {
|
||||
public static void main(String[] args)
|
||||
throws IOException, ConstantPoolException, InvalidDescriptor, InvalidIndex {
|
||||
new RedundantByteCodeInArrayTest()
|
||||
.checkClassFile(new File(System.getProperty("test.classes", "."),
|
||||
RedundantByteCodeInArrayTest.class.getName() + ".class"));
|
||||
}
|
||||
|
||||
void arrMethod(int[] array, int p, int inc) {
|
||||
array[p] += inc;
|
||||
}
|
||||
|
||||
void checkClassFile(File file)
|
||||
throws IOException, ConstantPoolException, InvalidDescriptor, InvalidIndex {
|
||||
ClassFile classFile = ClassFile.read(file);
|
||||
ConstantPool constantPool = classFile.constant_pool;
|
||||
|
||||
//lets get all the methods in the class file.
|
||||
for (Method method : classFile.methods) {
|
||||
if (method.getName(constantPool).equals("arrMethod")) {
|
||||
Code_attribute code = (Code_attribute) method.attributes
|
||||
.get(Attribute.Code);
|
||||
if (code.max_locals > 4)
|
||||
throw new AssertionError("Too many locals for method arrMethod");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue