[PATCH] 8147527: Non-optimal code generated for postfix unary operators
Jan Lahoda
jan.lahoda at oracle.com
Thu Nov 3 13:45:57 UTC 2016
Hi Bernard,
Thanks for looking at this. For the avoidance of the modification of
tree.selected in Lower.visitSelect, what I meant was more that we would
create a new instance of JCFieldAccess, to carry the new select. Might
be easier that trying to work with the existing select.
I think it might be useful to have a set of tests covering the usecases,
so that we can more easily test patches (we will need tests eventually
anyway). I can look at that, unless you want to.
On 3.11.2016 13:28, bsrbnd wrote:
> Here is a short summary of this issue.
>
> 1) The problem related to issue 8143388 changeset (which is probably a
> new issue) discovered here:
> http://mail.openjdk.java.net/pipermail/compiler-dev/2016-October/010452.html
>
> is solved by the fix proposed here:
> http://mail.openjdk.java.net/pipermail/compiler-dev/2016-October/010487.html
>
> 2) The optimization expected in issue 8147527 is obtained with the
> following patch:
> http://mail.openjdk.java.net/pipermail/compiler-dev/2016-October/010448.html
>
> 3) I've added two additional checks (for the optimization) that verify
> if this$n:
> a) is synthetic
> b) matches the exact regex "this\$\d+" (instead of startsWith("this$")).
Offhand, this seems a little bit scary.
Jan
>
> The following example (derived from issue 8147527 with some additional
> cases) shows the result of the optimization:
>
> class Issue8147527 extends p.P {
> public static void main(String[] args) {
> Issue8147527 self = new Issue8147527();
> self.testAll();
> }
>
> private void testAll() {
> System.out.println(test());
> System.out.println(i);
>
> Inner in = new Inner();
> System.out.println(in.test());
> System.out.println(i);
>
> System.out.println(testParent());
> System.out.println(super.j);
>
> System.out.println(in.testParent());
> System.out.println(super.j);
> }
>
> Integer i=0;
> private Integer test() {
> i++;
> return this.i++;
> }
> private Integer testParent() {
> j++;
> return super.j++;
> }
>
> class Inner {
> private Integer test() {
> i++;
> return Issue8147527.this.i++;
> }
> private Integer testParent() {
> j++;
> return Issue8147527.super.j++;
> }
> }
> }
>
> package p;
>
> public class P {
> protected Integer j=20;
> }
>
> The desugared code looks like this (as expected in issue 8147527):
>
> class Issue8147527$Inner {
>
> private Integer test() {
> (let /*synthetic*/ final Integer $647942 = this$0.i in (let
> /*synthetic*/ final Integer $29472086 = this$0.i =
> Integer.valueOf((int)(this$0.i.intValue() + 1)) in $647942));
> return (let /*synthetic*/ final Integer $8278885 =
> (Integer)this$0.i in (let /*synthetic*/ final Integer $16578475 =
> this$0.i = Integer.valueOf((int)(this$0.i.intValue() + 1)) in
> $8278885));
> }
>
> private Integer testParent() {
> (let /*synthetic*/ final Integer $798605 =
> Issue8147527.access$200(this$0) in (let /*synthetic*/ final Integer
> $5995540 = Issue8147527.access$302(this$0,
> Integer.valueOf((int)(Issue8147527.access$400(this$0).intValue() +
> 1))) in $798605));
> return (let /*synthetic*/ final Integer $33017129 =
> (Integer)Issue8147527.access$501(this$0) in (let /*synthetic*/ final
> Integer $25625839 = Issue8147527.access$603(this$0,
> Integer.valueOf((int)(Issue8147527.access$701(this$0).intValue() +
> 1))) in $33017129));
> }
> }
>
> class Issue8147527 extends p.P {
>
> private Integer test() {
> (let /*synthetic*/ final Integer $21617 = i in (let
> /*synthetic*/ final Integer $20066205 = i =
> Integer.valueOf((int)(i.intValue() + 1)) in $21617));
> return (let /*synthetic*/ final Integer $32619253 =
> (Integer)this.i in (let /*synthetic*/ final Integer $27442939 = this.i
> = Integer.valueOf((int)(this.i.intValue() + 1)) in $32619253));
> }
>
> private Integer testParent() {
> (let /*synthetic*/ final Integer $4208220 = j in (let
> /*synthetic*/ final Integer $32359562 = j =
> Integer.valueOf((int)(j.intValue() + 1)) in $4208220));
> return (let /*synthetic*/ final Integer $20049680 =
> (Integer)super.j in (let /*synthetic*/ final Integer $28368043 =
> super.j = Integer.valueOf((int)(super.j.intValue() + 1)) in
> $20049680));
> }
> }
>
> Putting all (1, 2 and 3) together gives the patch below.
>
> Bernard
>
> diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
> b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
> --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
> +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
> @@ -926,7 +926,7 @@
> ClassSymbol accOwner = refSuper && protAccess
> // For access via qualified super (T.super.x), place the
> // access symbol on T.
> - ? (ClassSymbol)((JCFieldAccess) tree).selected.type.tsym
> + ? (ClassSymbol)((JCFieldAccess)((JCFieldAccess)
> tree).selected).selected.type.tsym
> // Otherwise pretend that the owner of an accessed
> // protected symbol is the enclosing class of the current
> // class which is a subclass of the symbol's owner.
> @@ -1100,6 +1100,7 @@
> tree = make.at(tree.pos).Ident(sym);
> }
> JCExpression base = (tree.hasTag(SELECT)) ? ((JCFieldAccess)
> tree).selected : null;
> + base = refSuper ? translate(base) : base;
> switch (sym.kind) {
> case TYP:
> if (sym.owner.kind != PCK) {
> @@ -2209,16 +2210,20 @@
> */
> JCExpression abstractRval(JCExpression rval, Type type,
> TreeBuilder builder) {
> rval = TreeInfo.skipParens(rval);
> + boolean synthetic = false;
> switch (rval.getTag()) {
> case LITERAL:
> return builder.build(rval);
> case IDENT:
> JCIdent id = (JCIdent) rval;
> + synthetic = (id.sym.flags() & SYNTHETIC) != 0;
> if ((id.sym.flags() & FINAL) != 0 && id.sym.owner.kind == MTH)
> return builder.build(rval);
> }
> Name name = TreeInfo.name(rval);
> - if (name == names._super)
> + String thisDollarN = names._this + "\\" +
> target.syntheticNameChar() + "\\d+";
> + boolean matchesThisDollar = synthetic && name != null &&
> name.toString().matches(thisDollarN);
> + if (name == names._super || name == names._this || matchesThisDollar)
> return builder.build(rval);
> VarSymbol var =
> new VarSymbol(FINAL|SYNTHETIC,
> @@ -3837,7 +3842,7 @@
> tree.selected.hasTag(SELECT) &&
> TreeInfo.name(tree.selected) == names._super &&
> !types.isDirectSuperInterface(((JCFieldAccess)tree.selected).selected.type.tsym,
> currentClass);
> - tree.selected = translate(tree.selected);
> + if (!qualifiedSuperAccess) tree.selected = translate(tree.selected);
> if (tree.name == names._class) {
> result = classOf(tree.selected);
> }
>
More information about the compiler-dev
mailing list