Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-23 Thread Aleksey Shipilev
Yes, sorry, pushing these two: http://cr.openjdk.java.net/~shade/8150180/webrev.jdk.02 http://cr.openjdk.java.net/~shade/8150180/webrev.hs.02/ -Aleksey On 02/23/2016 05:39 PM, Claes Redestad wrote: > Looks good to me, but I assume you mean to push >

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-23 Thread Claes Redestad
Looks good to me, but I assume you mean to push http://cr.openjdk.java.net/~shade/8150180/webrev.jdk.02 (which looks even better)? Thanks! /Claes On 2016-02-23 11:24, Aleksey Shipilev wrote: On 02/19/2016 05:42 PM, Vladimir Ivanov wrote:

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-23 Thread Aleksey Shipilev
On 02/19/2016 05:42 PM, Vladimir Ivanov wrote: >>http://cr.openjdk.java.net/~shade/8150180/webrev.jdk.01/ >>http://cr.openjdk.java.net/~shade/8150180/webrev.hs.02/ > Looks good. A few more reviews, please? Cheers, -Aleksey

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread Christian Thalinger
> On Feb 19, 2016, at 1:22 PM, Aleksey Shipilev > wrote: > > On 02/20/2016 01:40 AM, Christian Thalinger wrote: >>> On Feb 19, 2016, at 9:03 AM, John Rose wrote: >>> On Feb 19, 2016, at 9:57 AM, Christian Thalinger >>>

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread Aleksey Shipilev
On 02/20/2016 01:40 AM, Christian Thalinger wrote: >> On Feb 19, 2016, at 9:03 AM, John Rose wrote: >> On Feb 19, 2016, at 9:57 AM, Christian Thalinger >> > >> wrote: >>> Why don’t you change the

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread John Rose
On Feb 19, 2016, at 2:40 PM, Christian Thalinger wrote: > >> >> On Feb 19, 2016, at 9:03 AM, John Rose > > wrote: >> >> On Feb 19, 2016, at 9:57 AM, Christian Thalinger >>

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread Christian Thalinger
> On Feb 19, 2016, at 9:03 AM, John Rose wrote: > > On Feb 19, 2016, at 9:57 AM, Christian Thalinger > > > wrote: >> >> Why don’t you change the values to: >> >>static final byte LATIN1 = 1;

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread John Rose
On Feb 19, 2016, at 9:57 AM, Christian Thalinger wrote: > > Why don’t you change the values to: > >static final byte LATIN1 = 1; >static final byte UTF16 = 2; Not sure what you are asking, but here's my take on why 0,1 is the (slight) winner. The

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread Christian Thalinger
> On Feb 19, 2016, at 4:05 AM, Peter Levart wrote: > > Hi, > > Just a stupid question. > > Your comment in String: > > 140 * @implNote Note this field is not {@link Stable}, because we want > 141 * LATIN1 (0) coder to fold too. {@link Stable} would not allow

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread Aleksey Shipilev
On 02/19/2016 05:36 PM, Peter Levart wrote: > Yes, I understand the general principle of @Stable design and the role > of default value in @Stable fields. But given that: > > But never mind. This is just me thinking loud. Sure. The answer to those questions about @Stable is, as always: it works

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread Vladimir Ivanov
http://cr.openjdk.java.net/~shade/8150180/webrev.jdk.01/ http://cr.openjdk.java.net/~shade/8150180/webrev.hs.02/ Looks good. Best regards, Vladimir Ivanov

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread Peter Levart
On 02/19/2016 03:07 PM, Aleksey Shipilev wrote: Hi Peter, On 02/19/2016 05:05 PM, Peter Levart wrote: Your comment in String: 140 * @implNote Note this field is not {@link Stable}, because we want 141 * LATIN1 (0) coder to fold too. {@link Stable} would not allow that, 142

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread Aleksey Shipilev
On 02/19/2016 03:42 PM, Vladimir Ivanov wrote: >>http://cr.openjdk.java.net/~shade/8150180/webrev.hs.01/ > > Why don't you check that index is also a constant? Otherwise, C2 can't > constant fold the loads. > > + if (value->is_Con()) { > +return false; > + } Right. Without that,

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread Aleksey Shipilev
Hi Peter, On 02/19/2016 05:05 PM, Peter Levart wrote: > Your comment in String: > > 140 * @implNote Note this field is not {@link Stable}, because we > want > 141 * LATIN1 (0) coder to fold too. {@link Stable} would not allow > that, > 142 * as it reserves the default value as

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread Peter Levart
Hi, Just a stupid question. Your comment in String: 140 * @implNote Note this field is not {@link Stable}, because we want 141 * LATIN1 (0) coder to fold too. {@link Stable} would not allow that, 142 * as it reserves the default value as "not initialized" value. 143

Re: RFR (S) 8150180: String.value contents should be trusted

2016-02-19 Thread Vladimir Ivanov
http://cr.openjdk.java.net/~shade/8150180/webrev.hs.01/ Why don't you check that index is also a constant? Otherwise, C2 can't constant fold the loads. + if (value->is_Con()) { +return false; + } Best regards, Vladimir Ivanov