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

RFS (XS): 7169934: pow(x,y) or x64 computes incorrect result when x<0 and y is an odd integer

Roland Westrelin — 2012-05-18T11:16:05

http://cr.openjdk.java.net/~roland/7169934/webrev.00/

A last minute code clean-up broke the check for the parity of y in pow(x,y) in debug builds.

Roland.

hg: hsx/hotspot-comp/langtools: Added tag jdk8-b39 for changeseta9f547c218d9

2012-05-18T03:42:05

Changeset: 8b869afd2eb4
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/langtools/rev/8b869afd2eb4

Added tag jdk8-b39 for changeset a9f547c218d9

! .hgtags

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

2012-05-18T03:39:30

Changeset: c2d9166f3284
Author:    ihse
Date:      2012-05-11 08:21 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/jdk/rev/c2d9166f3284

7168208: Change use of < at > in one sed command involving paths to different character
Reviewed-by: ohair

! make/common/Release.gmk

Changeset: 8d665b69ebf1
Author:    mfang
Date:      2012-05-15 11:46 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/jdk/rev/8d665b69ebf1

7157855: jvisualvm.1 not included in binaries
Reviewed-by: katleman, thurka

! make/common/Release.gmk

Changeset: b6f529117521
Author:    katleman
Date:      2012-05-16 22:07 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/jdk/rev/b6f529117521

Merge


Changeset: 47cd90bf0f66
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/jdk/rev/47cd90bf0f66

Added tag jdk8-b39 for changeset b6f529117521

! .hgtags

hg: hsx/hotspot-comp/jaxws: Added tag jdk8-b39 for changeset7f6b44fd3034

2012-05-18T03:39:16

Changeset: 09a0ddda03cb
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/jaxws/rev/09a0ddda03cb

Added tag jdk8-b39 for changeset 7f6b44fd3034

! .hgtags

hg: hsx/hotspot-comp/jaxp: Added tag jdk8-b39 for changesetf95fdbe525c8

2012-05-18T03:39:04

Changeset: 9ecfdbd6aed4
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/jaxp/rev/9ecfdbd6aed4

Added tag jdk8-b39 for changeset f95fdbe525c8

! .hgtags

hg: hsx/hotspot-comp/corba: Added tag jdk8-b39 for changeset785af00e2827

2012-05-18T03:38:56

Changeset: 56d030e5035f
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/corba/rev/56d030e5035f

Added tag jdk8-b39 for changeset 785af00e2827

! .hgtags

hg: hsx/hotspot-comp: 4 new changesets

2012-05-18T03:38:50

Changeset: 955a3e8ed4f0
Author:    ohair
Date:      2012-05-10 08:26 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/rev/955a3e8ed4f0

7167593: Changed get_source.sh to allow for getting full oracle jdk repo forest
Reviewed-by: erikj, asaha, chegar, sla, dholmes, mbykov, coleenp

! get_source.sh
! make/scripts/hgforest.sh

Changeset: 8a4e92c10a9a
Author:    ohair
Date:      2012-05-11 17:52 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/rev/8a4e92c10a9a

7167976: Fix broken get_source.sh script
Reviewed-by: tbell

! make/scripts/hgforest.sh

Changeset: 8927dd68aee3
Author:    katleman
Date:      2012-05-16 22:06 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/rev/8927dd68aee3

Merge


Changeset: a2b2d435f1d2
Author:    katleman
Date:      2012-05-17 06:20 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/rev/a2b2d435f1d2

Added tag jdk8-b39 for changeset 8927dd68aee3

! .hgtags

Re: Request for review (XS): crash in C2 when using-XX:+CountCompiledCalls

Christian Thalinger — 2012-05-18T00:40:50

Looks good.  -- Chris

On May 16, 2012, at 6:02 AM, Krystal Mok wrote:

Re: [concurrency-interest] a volatile bug?

Christian Thalinger — 2012-05-16T23:42:22

On May 16, 2012, at 2:22 PM, Aleksey Shipilev wrote:


Good investigation.  That seems to be the case.


I can confirm the bug and I have a fix for it.

Re: [concurrency-interest] a volatile bug?

Aleksey Shipilev — 2012-05-16T21:22:17

Well, I do not want to sound alarming, but... if I understand the C1
code correctly, then C1 GVN does not account prior volatile reads at
all. I can not find any code in C1 GVN code which actually prevents
killing second non-volatile read after volatile one, which is required
by JMM semantics.

I think I'll stop here. The impact of this issue is limited, given
most of the guys run -server (even by default on most machines), so
there is always the workaround for running with -server. Also, I would
*speculate* turning off GVN with -XX:-UseGlobalValueNumbering when
running with -client is still a workaround, but kind of insane one,
since it can *severely* degrade performance.

Words of wisdom: I'm using this command-line to print out GVN tracing:
$ ~/Install/jdk7u4/fastdebug/bin/java -XX:+PrintCompilation
-XX:+PrintDominators -XX:+PrintCompilation  -XX:+PrintValueNumbering
-Xbatch -XX:CompileOnly=Test.$1,Test -client  Test 2>&1 | tee asm.log

Can anyone more proficient in C1 code confirm this?

-Aleksey.

On Th

Re: [concurrency-interest] a volatile bug?

Vitaly Davidovich — 2012-05-16T20:34:17

I looked at the assembly on SO again (the pastebin link) and it seems to be
correct actually: after 'a' is cmp'ed against zero, 'b' is read from
memory.  But now someone is saying there that it sometimes generates the
correct assembly and other times not - very strange.
0x025bd2b9: cmp $0x0,%edx

30. 0x025bd2bc: je 0x025bd2a8 ;

32. 0x025bd2be: mov $0x147062e8,%edx ; {oop('test/TestVolatile')}

33. 0x025bd2c3: mov 0x1c4(%edx),%edx ;*getstatic b

34. ; - test.TestVolatile::run< at >10 (line 17)

35. 0x025bd2c9: cmp $0x0,%edx

Sent from my phone
On May 16, 2012 3:55 PM, "Aleksey Shipilev" <aleksey.shipilev-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org>
wrote:

Re: [concurrency-interest] a volatile bug?

Vitaly Davidovich — 2012-05-16T19:12:25

It can be a compiler (mis)optimization that causes this, and not x86 memory
ordering.

Someone posted the assembly output in the comments on SO and it does seem
like there's a place that loads 'b' from the stack rather than memory.
Hans' theory of CSE sounds plausible - can someone repro this without that
"int tt = b;" line?

Adding hotspot compiler guys in case they want to chime in.

Sent from my phone
On May 16, 2012 3:07 PM, "Aleksey Shipilev" <aleksey.shipilev-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org>
wrote:

Re: Request for review (XS): crash in C2 when using-XX:+CountCompiledCalls

Krystal Mok — 2012-05-16T16:13:41

Thank you, Vladimir :-)

- Kris

On Thu, May 17, 2012 at 12:12 AM, Vladimir Kozlov <
vladimir.kozlov-QHcLZuEGTsvQT0dZR+AlfA< at >public.gmane.org> wrote:

Re: Request for review (XS): crash in C2 when using-XX:+CountCompiledCalls

Vladimir Kozlov — 2012-05-16T16:12:53

Kris,

Changes looks good. I will push it tomorrow.

Thanks,
Vladimir

On 5/16/12 6:02 AM, Krystal Mok wrote:

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

Roland Westrelin — 2012-05-16T14:32:20

Ok. Thanks for the review.

Roland.

Request for review (XS): crash in C2 when using-XX:+CountCompiledCalls

Krystal Mok — 2012-05-16T13:02:02

Hi all,

Could I get some reviews for this patch, please?
https://gist.github.com/2710000#file_count_compiled_calls.patch

Description:

C2 may crash when compiling methods with -XX:+CountCompiledCalls turned on.
The cause is in Parse::count_compiled_calls(), where it made a TypeInstPtr
from a ciMethod:

const TypeInstPtr* addr_type = TypeInstPtr::make(method());

Since the klass of a ciMethod is a ciMethodKlass, which isn't a
ciInstanceKlass, an assertion is hit later in
Compile::flatten_alias_type(), where it's expecting a TypeInstPtr to have a
ciInstanceKlass:

ciInstanceKlass *k = to->klass()->as_instance_klass();

An example of the crash is available here: https://gist.github.com/2710000
In the example, I started a Groovy shell with -XX:+CountCompiledCalls set,
and it crashed quickly when compiling java.lang.String.charAt().

The fix is to use TypeOopPtr::make_from_constant(method()) instead
of TypeInstPtr::make(method()).
I did check the hg history, and looks like it's been like this since
duke< at >0.Wonde

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

Vladimir Kozlov — 2012-05-15T18:23:32

In c1_LIRAssembler_sparc.cpp move assert(code == lir_xchg) to the beginning of method since on sparc we have only swap 
for all types and change "xadd for oops" --> "no xadd on sparc".

In c1_LIR.cpp fix comment:

+        // destroy inputs. On the other platform that implements those
+        // (x86, sparc), the extra constrainsts are armless.
----
+        // destroy inputs. On other platforms that implement those
+        // (x86, sparc), the extra constraints are harmless.

In escape.cpp move add_obload_* call up with cases for GetAndSet and then fallthrough:

+    case Op_GetAndSetP:
+    case Op_GetAndSetN: {
+      add_objload_to_connection_graph(n, delayed_worklist);
+      // fallthrough
+    }
      case Op_StoreP:
      case Op_StoreN:
      case Op_StorePConditional:
      case Op_CompareAndSwapP:
      case Op_CompareAndSwapN: {

The rest looks good.

Vladimir


On 5/15/12 9:10 AM, Roland Westrelin wrote:

RFR (M): 7023898: Intrinsify AtomicLongFieldUpdater.getAndIncrement()

Roland Westrelin — 2012-05-15T16:10:06

http://cr.openjdk.java.net/~roland/7023898/

This change provides intrinsics when optimized instruction sequences for the new Unsafe methods below exist (the change to the libraries will get in as a separate piece of work).

Roland.

   /**
    * Atomically update Java variable by <tt>delta</tt> returning
    * the previous value.
    * < at >return the previous value
    */
   public int getAndAddInt(Object o, long offset, int delta) {
       for (;;) {
           int current = getInt(obj, offset);
           int next = current + delta;
           if (compareAndSwapInt(obj, offset, current, next)) {
               return current;
           }
       }
   }

   /**
    * Atomically update Java variable by <tt>delta</tt> returning
    * the previous value.
    * < at >return the previous value
    */
   public long getAndAddLong(Object o, long offset, long delta) {
       for (;;) {
           long current = getLongVolatile(obj, offset);
           long next = current + delta;
           if (compareAndSwapLong(obj, off

Re: RFR (S): 7167254: Crash on OSX in Enumerator.nextElement() withcompressed oops

Dmitry Samersoff — 2012-05-15T13:34:56

Looks good for me.

On 2012-05-15 17:22, Roland Westrelin wrote:

hg: hsx/hotspot-comp/hotspot: 7133857: exp() and pow() should use thex87 ISA on x86

2012-05-15T11:26:56

Changeset: 6759698e3140
Author:    roland
Date:      2012-05-15 10:10 +0200
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/6759698e3140

7133857: exp() and pow() should use the x87 ISA on x86
Summary: use x87 instructions to implement exp() and pow() in interpreter/c1/c2.
Reviewed-by: kvn, never, twisti

! src/cpu/sparc/vm/c1_LIRGenerator_sparc.cpp
! src/cpu/sparc/vm/interpreter_sparc.cpp
! src/cpu/x86/vm/assembler_x86.cpp
! src/cpu/x86/vm/assembler_x86.hpp
! src/cpu/x86/vm/c1_LIRAssembler_x86.cpp
! src/cpu/x86/vm/c1_LIRGenerator_x86.cpp
! src/cpu/x86/vm/c1_LinearScan_x86.cpp
! src/cpu/x86/vm/interpreter_x86_32.cpp
! src/cpu/x86/vm/interpreter_x86_64.cpp
! src/cpu/x86/vm/stubGenerator_x86_32.cpp
! src/cpu/x86/vm/stubGenerator_x86_64.cpp
! src/cpu/x86/vm/templateInterpreter_x86_32.cpp
! src/cpu/x86/vm/templateInterpreter_x86_64.cpp
! src/cpu/x86/vm/x86_32.ad
! src/cpu/x86/vm/x86_64.ad
! src/share/vm/c1/c1_GraphBuilder.cpp
! src/share/vm/c1/c1_LIR.cpp
! src/share/vm/c1/c1_LIR.hpp
! src/shar

hg: hsx/hotspot-comp/hotspot: 6924259: RemoveString.count/String.offset

2012-05-14T22:37:36

Changeset: 8f972594effc
Author:    kvn
Date:      2012-05-14 09:36 -0700
URL:       http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/8f972594effc

6924259: Remove String.count/String.offset
Summary: Allow a version of String class that doesn't have count and offset fields.
Reviewed-by: never, coleenp

! src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp
! src/cpu/x86/vm/c1_LIRAssembler_x86.cpp
! src/share/vm/classfile/javaClasses.cpp
! src/share/vm/classfile/javaClasses.hpp
! src/share/vm/classfile/systemDictionary.cpp
! src/share/vm/classfile/vmSymbols.hpp
! src/share/vm/memory/dump.cpp
! src/share/vm/opto/graphKit.cpp
! src/share/vm/opto/graphKit.hpp
! src/share/vm/opto/library_call.cpp
! src/share/vm/opto/stringopts.cpp
! src/share/vm/opto/stringopts.hpp