review (M) for 6892658: C2 should optimize some stringbuilder patterns
Vladimir.Kozlov at Sun.COM
Wed Nov 11 10:36:13 PST 2009
Tom Rodriguez wrote:
>> Also _begin instead of other->_begin
>> in result->set_allocation(other->_begin)
> _begin is must be the earliest JVMState of the pattern and other->_begin has to be earlier than _begin otherwise the couldn't be merged so I can't just swap them around.
Then I don't get how you optimize next code:
and I don't see any checks that other->_begin dominates _begin.
>> There are several places where you do next check,
>> may be you can factor it in a separate function:
>> method->holder() == C->env()->StringBuilder_klass() ||
>> method->holder() == C->env()->StringBuffer_klass()
> I'm not sure factoring it out would be better.
>> May be also verify has_stringbuilder() in PhaseStringOpts().
OK, I see that caller code of PhaseStringOpts() has has_stringbuilder()
>> Also this coalesce code will not work if "other" is used by
>> several sc/arguments since you removed it from the list after
>> first match and merge. For example:
>> String s0 = new SB().append(1)...toString();
>> String s1 = new SB().append(s0).append(s0).toString();
>> I would keep it and always replace "c" with merged
>> (you need to modify StringConcat::merge() as I pointed above).
>> The "o" will be removed automatically if there are no other uses.
> I don't want to support that. I don't think that's an interesting pattern. It would also require rewriting the management of the control and trap lists and I don't want to get into that.
>> I will look on copy_string() and related methods tomorrow.
More information about the hotspot-compiler-dev