review (M) for 6892658: C2 should optimize some stringbuilder patterns

[Tom Rodriguez]

I've updated the webrev with all these changes.

tom

On Nov 11, 2009, at 8:36 AM, Vladimir Kozlov wrote:

> Tom Rodriguez wrote:
>>> 
>>> the_MIN_VALUE_string may be should be the_MIN_INT_VALUE_string
>> I was trying to mimic Integer.MIN_VALUE.  If you want int in there then how about the_min_jint_string?
> 
> Agree with the_min_jint_string.
> 
>>> c2_globals.hpp
>>> I thought we will use experimental for OptimizeStringConcat.
>> I guess we could.  I don't really like experimental much but I'm not completely against it.
> 
> I would prefer experimental.
> 
>>> doCall.cpp
>>> I saw cases when append methods were not inlined because they were
>>> already compiled into "big" compiled method. You call for_late_inline()
>>> after ok_to_inline(), so may be we should relax that condition for
>>> OptimizeStringConcat case.
>> The code doesn't care whether the interesting methods are inlined or not.  If the methods would have been inlined then we register a late inline for them.  If they wouldn't have been inlined then we tell the DirectCallGenerator to use separate io projs so that we can properly find all the edges if we need to replace it.  THe late inlining logic is mainly about giving us the same result if we don't end up performing the optimization.
> 
> You are right. The code delays inlining, not trying to inline.
> 
>>> Should you check that safepoint has > jvms->argoff() inputs?:
>>> Node* receiver = jvms->map()->in(jvms->argoff() + 1);
>> I guess I could but if there aren't enough then the call itself is malformed and we'll die later won't we?  Do you think i should?
> 
> Leave it as it is.
> 
>>> macro.cpp
>>> Can you print array only when it is array?:
>>> +     log->head("eliminate_allocation %s type='%d'",
>>> +               alloc->is_AllocateArray() ? "array" : "", log->identify(tklass->klass()));
>>> 
>>> the same with "lock":"unlock"
>> I could but that's not very good xml.  I think I'll just leave that out since arrayness should be discerned from the klass.  I'd prefer to leave it as it for lock though.
> 
> Leave it as it is since it is better for XML.
> 
> Thanks,
> Vladimir
> 
>> tom
>>> Vladimir
>>> 
>>> Tom Rodriguez wrote:
>>>> http://cr.openjdk.java.net/~never/6892658/