gmane.comp.java.openjdk.hotspot.compiler.devel

hg: hsx/hotspot-comp/hotspot: 7170145: C1 doesn't respect the JMMwith volatile field loads

2012-05-25T21:52:18

Changeset: 4d8787136e08
Author:    twisti
Date:      2012-05-25 11:39 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/4d8787136e08

7170145: C1 doesn't respect the JMM with volatile field loads
Reviewed-by: kvn, roland

! src/share/vm/c1/c1_ValueMap.hpp

hg: hsx/hotspot-comp/hotspot: 7170463: C2 should recognize"obj.getClass() == A.class" code pattern

2012-05-25T18:05:47

Changeset: 8f6ce6f1049b
Author:    kvn
Date:      2012-05-25 07:53 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/8f6ce6f1049b

7170463: C2 should recognize "obj.getClass() == A.class" code pattern
Summary: optimize this code pattern obj.getClass() == A.class.
Reviewed-by: jrose, kvn
Contributed-by: Krystal Mok <sajia-3b8fjiQLQpfQT0dZR+AlfA< at >public.gmane.org>

! src/share/vm/opto/parse.hpp
! src/share/vm/opto/parse2.cpp
! src/share/vm/opto/subnode.cpp

hg: hsx/hotspot-comp/langtools: 6 new changesets

2012-05-25T13:51:31

Changeset: d10db3576c08
Author:    ksrini
Date:      2012-05-04 07:55 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/langtools/rev/d10db3576c08

7166010: (javac) JavacMessager incorrectly restores log source file
Reviewed-by: jjg
Contributed-by: jan.lahoda-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org

! src/share/classes/com/sun/tools/javac/processing/JavacMessager.java
+ test/tools/javac/processing/messager/MessagerDiags.java

Changeset: 833bab705918
Author:    ksrini
Date:      2012-05-10 12:32 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/langtools/rev/833bab705918

7159445: (javac) emits inaccurate diagnostics for enhanced for-loops
Reviewed-by: jjg
Contributed-by: jan.lahoda-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org

! src/share/classes/com/sun/tools/javac/parser/JavacParser.java
! src/share/classes/com/sun/tools/javac/resources/compiler.properties
+ test/tools/javac/diags/examples/ForeachBadInitialization.java
! test/tools/javac/parser/JavacParserTest.java

Changeset: 96a8278e323c

hg: hsx/hotspot-comp/hotspot: 29 new changesets

2012-05-25T05:34:53

Changeset: 35e504cb49a6
Author:    collins
Date:      2012-05-11 11:30 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/35e504cb49a6

7167625: Adjustments for SE-Embedded build process
Summary: Simple change to the SE-Embedded build rules that should not affect any other OpenJDK users.
Reviewed-by: kvn, dholmes

! make/linux/makefiles/vm.make
! src/share/vm/runtime/arguments.cpp

Changeset: fada85d11d92
Author:    jprovino
Date:      2012-05-16 13:33 -0400
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/fada85d11d92

Merge


Changeset: de0cc3dd9f10
Author:    kvn
Date:      2012-05-17 09:50 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/de0cc3dd9f10

Merge


Changeset: 3a22b77e755a
Author:    brutisso
Date:      2012-05-14 17:32 +0200
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/3a22b77e755a

7161545: G1: Minor cleanups to the G1 logging
Summary: Rename "to-space-overflow" to "to-space-exhausted", Introduce one decimal

hg: hsx/hotspot-comp/jdk: 71 new changesets

2012-05-25T03:38:34

Changeset: 7c4eed4b6c19
Author:    bae
Date:      2012-05-21 14:04 +0400
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/jdk/rev/7c4eed4b6c19

7124400: [macosx] CGraphicsDevice.getConfigurations() returns reference to member (does not copy configs)
Reviewed-by: anthony, kizune

! src/macosx/classes/sun/awt/CGraphicsDevice.java
! test/java/awt/GraphicsDevice/CloneConfigsTest.java

Changeset: 416b3a498e71
Author:    bae
Date:      2012-05-21 14:53 +0400
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/jdk/rev/416b3a498e71

7154088: [macosx] Regression: Component.createImage do not inherits component attributes
Reviewed-by: art, kizune

! src/macosx/classes/sun/lwawt/LWComponentPeer.java

Changeset: 1b90a0113359
Author:    lana
Date:      2012-05-21 11:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/jdk/rev/1b90a0113359

Merge


Changeset: c31eeeda3ed1
Author:    serb
Date:      2012-05-03 18:29 +0400
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/jdk/rev/c31eeeda3ed1

71606

hg: hsx/hotspot-comp/jaxws: Added tag jdk8-b40 for changeset09a0ddda03cb

2012-05-25T03:37:23

Changeset: f2072b164b05
Author:    katleman
Date:      2012-05-24 16:15 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/jaxws/rev/f2072b164b05

Added tag jdk8-b40 for changeset 09a0ddda03cb

! .hgtags

hg: hsx/hotspot-comp/jaxp: Added tag jdk8-b40 for changeset9ecfdbd6aed4

2012-05-25T03:37:11

Changeset: 6f5c0e17415d
Author:    katleman
Date:      2012-05-24 16:15 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/jaxp/rev/6f5c0e17415d

Added tag jdk8-b40 for changeset 9ecfdbd6aed4

! .hgtags

hg: hsx/hotspot-comp/corba: Added tag jdk8-b40 for changeset56d030e5035f

2012-05-25T03:37:04

Changeset: 113f0d5f0a08
Author:    katleman
Date:      2012-05-24 16:15 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/corba/rev/113f0d5f0a08

Added tag jdk8-b40 for changeset 56d030e5035f

! .hgtags

hg: hsx/hotspot-comp: Added tag jdk8-b40 for changeset a2b2d435f1d2

2012-05-25T03:36:58

Changeset: 1a8c7c530f8a
Author:    katleman
Date:      2012-05-24 16:15 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/rev/1a8c7c530f8a

Added tag jdk8-b40 for changeset a2b2d435f1d2

! .hgtags

Re: Request for reviews (M): 7170463: C2 should recognize"obj.getClass() == A.class" code pattern

Krystal Mok — 2012-05-24T22:15:07

Hi John,

Thanks a lot! I added the join() in my patch but I wasn't sure it was nessecary; I debugged through it for a couple of cases and the resulting type seemed to be what I wanted, so I added it. Now I'm getting it :-)

Thanks,
Kris

On 2012-5-25, at 3:23, John Rose <john.r.rose-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org> wrote:

Re: Request for reviews (M): 7170463: C2 should recognize"obj.getClass() == A.class" code pattern

John Rose — 2012-05-24T19:23:58



Kris, the join is important locally to "climb the hill" to a better GVN type assignment; you don't want to lose type information you already have.
The join does a set union (normalized by upper bound in lattice) of assertions about the type.  There may be assertions unique to either source of information, and we want the output to cover all known assertions.

(In the literature the you will often read "meet" where we say "join".  But we stand on our heads here in C2.  When type domains are narrowed against each other, that is traditionally called a meet.  But conversely we can regard the assertion sets over type domains, and those are widened exactly when the value sets are narrowed, hence the name join.)

Globally, if you drop assertions on the floor, you may keep climbing to two distinct hilltops, lose confluence, and fail to terminate in the optimizer.  Here's a good summary of the global confluence property we rely on:  http://en.wikipedia.org/wiki/Abstract_rewriting_system

In this particular case, t

Re: Request for reviews (M): 7170463: C2 should recognize"obj.getClass() == A.class" code pattern

John Rose — 2012-05-24T19:22:16


Yes; it's just another guard for Top.  (How many times have we forgotten that guard? :-)

— John

Re: Request for reviews (M): 7170463: C2 should recognize"obj.getClass() == A.class" code pattern

Vladimir Kozlov — 2012-05-24T17:32:59

Yes, it could but the cast is pinned to True path which is dead (not executed) 
in such case (we don't change control flow in this code).

But current code will assert in TOP case during call to is_oopptr() which is not 
good:

obj_type->join(con_type)->is_oopptr()

I am changing this code so it will skip TOP case:

   const TypeOopPtr* tboth = obj_type->join(con_type)->isa_oopptr();
   if (tboth != NULL && tboth != obj_type) {

Thanks,
Vladimir

Re: Request for reviews (M): 7170463: C2 should recognize"obj.getClass() == A.class" code pattern

Krystal Mok — 2012-05-24T08:28:27

Hi Vladimir and John,

Thanks for polishing the changes!
Comments inline below:

On Thu, May 24, 2012 at 1:02 PM, John Rose <john.r.rose-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org> wrote:

for the catch!


of sharpen_type_after_if() with the new and old code mixed together; now it
looks nice.



Thanks,
Kris

Re: Request for reviews (M): 7170463: C2 should recognize"obj.getClass() == A.class" code pattern

John Rose — 2012-05-24T05:02:47


That's fine; I like it.

New question:  Can the following call return Top?
  obj_type->join(con_type)

E.g.,
  Number x = 5;
  if (x.getClass() == String.class)
    System.out.println("fail!");

Perhaps the compare will short-circuit before that happens.  If it doesn't, we'll get assertion failures or worse.

— John

Re: Request for reviews (M): 7170463: C2 should recognize"obj.getClass() == A.class" code pattern

Vladimir Kozlov — 2012-05-23T23:47:00

I updated webrev:

http://cr.openjdk.java.net/~kvn/7170463/webrev.01

I found additional issue with original changes. They accidentally removed 
optimization which replaces LoadKlass node with ConP (klass type) in map. Next 
code will not be executed:

    cast = con;  // Replace non-constant val by con.

New code should do separate map update of object which klass is compared.

John Rose wrote:

Done.


Base class TypeOopPtr is java pointer type and it currently includes 
TypeKlassPtr type until PermGen is removed. So for now I leave these checks as 
it is and late we can replace them with tp->isa_oopptr().

Vladimir

Re: RFR (M): 7023898: IntrinsifyAtomicLongFieldUpdater.getAndIncrement()

Roland Westrelin — 2012-05-23T14:19:08

Hi John,

For 1. et 2. below:


You're commenting this code change:

2866   if ((kind == xchg || kind == xadd) && type == T_LONG) {
2867     push_pair(load_store);
2868   } else {
2869     push(load_store);
2870   }

right?

Then for 2.: when kind == cmpxchg, then the else part is executed so load_store is pushed as a result.

and for 1.: when type is T_LONG for xchg or xadd, a pair is pushed but for cmpxchg a single result is pushed. So push_node wouldn't work, right?


Ok.


Ok.

Roland.

Re: Request for reviews (XS): 7170145: C1 doesn't respect the JMMwith volatile field loads

Roland Westrelin — 2012-05-23T08:49:16

Looks ok to me.

Roland.

Re: Request for reviews (XS): 7170145: C1 doesn't respect the JMMwith volatile field loads

Christian Thalinger — 2012-05-23T02:11:15

On May 22, 2012, at 6:30 PM, Mikael Vidstedt wrote:


Apparently I had too many versions of that text.  Thanks.

Re: Request for reviews (XS): 7170145: C1 doesn't respect the JMMwith volatile field loads

Mikael Vidstedt — 2012-05-23T01:30:26

Two typos:

145 // This is actually too strict and the JMM doesn't requires
146 // this is all cases (e.g. load a; volatile store b; load a)

should probably be "JMM doesn't _require_ this _in_ all cases...".

Cheers,
Mikael


On 2012-05-22 17:35, Christian Thalinger wrote:

Re: Request for reviews (XS): 7170145: C1 doesn't respect the JMMwith volatile field loads

Vladimir Kozlov — 2012-05-23T01:23:51

Good.

Vladimir

On 5/22/12 5:35 PM, Christian Thalinger wrote: