Request for reviews (XS): 7123926: Some CTW test crash: !_control.contains(ctrl)
Vladimir Kozlov
vladimir.kozlov at oracle.com
Fri Jul 13 17:14:51 PDT 2012
Thank you, Christian
Vladimir
Christian Thalinger wrote:
> Looks good. -- Chris
>
> On Jul 13, 2012, at 12:46 PM, Vladimir Kozlov wrote:
>
>> I updated webrev. I removed uncast() changes from this fix, we do it later. Tested with compiler and java/jang regression tests.
>>
>> http://cr.openjdk.java.net/~kvn/7123926/webrev.01
>>
>> Thanks,
>> Vladimir
>>
>> Vladimir Kozlov wrote:
>>> I looked again on my changes and, I think, the lines with push_int();continue; should be also under (arg->outcnt() == 1) condition. If we don't remove call to Integer.toString() - use its result. Otherwise we have to generate conversion from int to String by hand for each use (see int_getChars()). As result Integer.toString() code will not be folded by later optimizations for the case I showed in previous mail. I need to do more testing for this and will resend webrev later.
>>>> I'm a little bit worried about this part after seeing your fix for 7181658. Do you think it's safe to use uncast()
>>> It was another Vladimir (Ivanov) who fixed 7181658 :-)
>>>> this way, or should we use eqv_uncast() for comparison at the use sites of StringConcat::argument_uncast()?
>>> I think using uncast() in argument_uncast() is safe, it is used only in related code. The first time argument_uncast() is called to find a Proj node from SB.toString() call. Then that Proj node passed to merge() method as 'arg'. And in the merge() method argument_uncast() is called second time to compare with that passed 'arg'.
>>> Thanks,
>>> Vladimir
>>> On 7/5/12 8:23 PM, Krystal Mok wrote:
>>>> Comments inline:
>>>>
>>>> On Fri, Jul 6, 2012 at 10:38 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>> wrote:
>>>>
>>>> Thank you, Kris
>>>>
>>>>
>>>> Krystal Mok wrote:
>>>>
>>>> Hi Vladimir,
>>>>
>>>> foo(Integer intobj) {
>>>> String intstr = intobj.toString();
>>>> String str = intstr + ", " + intstr;
>>>>
>>>> What should be the desired behavior for the case above?
>>>> Is it all right to skip all Integer.toString() calls, and let later optimizations handle it?
>>>>
>>>>
>>>> You have only one toString() call. And it will be kept because it is referenced by debug info which is copied from
>>>> SB constructor to new char[] constructor (AllocateArray node). It is needed for deoptimization since the bytecode
>>>> will be executed after that call.
>>>>
>>>> I see. So that's what you meant in the original "Keep case". That makes sense.
>>>>
>>>> The next code is the case when Integer.toString() will be eliminated by later optimizations:
>>>>
>>>> StringBuilder sb = new StringBuilder();
>>>> String intstr = intobj.toString();
>>>> String str = sb.append(intstr).append(__intstr).toString();
>>>>
>>>> I also applied your uncast() suggestion in argument_uncast() method.
>>>>
>>>> I'm a little bit worried about this part after seeing your fix for 7181658. Do you think it's safe to use uncast() this
>>>> way, or should we use eqv_uncast() for comparison at the use sites of StringConcat::argument_uncast()?
>>>>
>>>> Thanks,
>>>> Kris
>>>>
>>>> And I fixed typos.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>>
>>>> And, two typos:
>>>>
>>>> 541 // chain. It's node could be eliminated only if it's result
>>>>
>>>> It's -> Its. Other occurrences of "it's" should also be replaced with "its".
>>>>
>>>> 543 // An other limitation: it should be used only once because
>>>>
>>>> An other -> Another.
>>>>
>>>> - Kris
>>>>
>>>> On Fri, Jul 6, 2012 at 4:37 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>
>>>> <mailto:vladimir.kozlov at __oracle.com <mailto:vladimir.kozlov at oracle.com>>> wrote:
>>>>
>>>> Vladimir Kozlov wrote:
>>>>
>>>> http://cr.openjdk.java.net/~____kvn/7123926/webrev <http://cr.openjdk.java.net/~__kvn/7123926/webrev>
>>>>
>>>> <http://cr.openjdk.java.net/~__kvn/7123926/webrev <http://cr.openjdk.java.net/~kvn/7123926/webrev>>
>>>>
>>>> 7123926: Some CTW test crash: !_control.contains(ctrl)
>>>>
>>>> Don't try to eliminate Integer::toString() call node during
>>>> String concatenation optimization if it's result has several uses.
>>>>
>>>> Eliminate case:
>>>> foo (Integer intob) {
>>>> String str = "int: " + intobj;
>>>>
>>>> Keep case:
>>>> foo (Integer intob) {
>>>> String intstr = intobj.toString();
>>>> String str = "int: " + intobj; // second use is in SB
>>>> allocation
>>>>
>>>>
>>>> Sorry, it should be: "int: " + intstr;
>>>>
>>>> Vladimir
>>>>
>>>>
>>>> debug info
>>>>
>>>> Tested with failed test.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>>
>>>>
>
More information about the hotspot-compiler-dev
mailing list