review (XS) for 6953539: after 6892658 c1 no longer inlines StringBuffer.append

[Tom Rodriguez]

On May 18, 2010, at 11:59 AM, Christian Thalinger wrote:

> On Tue, 2010-05-18 at 11:14 -0700, Tom Rodriguez wrote:
>> http://cr.openjdk.java.net/~never/6953539
>> 
>> 6953539: after 6892658 c1 no longer inlines StringBuffer.append
>> Reviewed-by:
>> 
>> 6892658 added intrinsic ids for several StringBuffer/StringBuilder
>> methods which triggered some long dormant logic in
>> try_inline_intrinsic which bails out of inlining if an intrinsic is
>> synchronized.  It really shouldn't be a full bailout, it should simply
>> return false to indicate that it didn't do anything.  Tested by
>> inspecting PrintInlining output.
> 
> INLINE_BAILOUT just sets _inline_bailout_msg and returns false.  And it
> seems _inline_bailout_msg is not read somewhere except printing it.  Why
> does this fix change the behavior?  I must be missing something.

You're right it is actually being inlined but the PrintInlining output implies that it's not.  Normally C1 only prints messages when it didn't inline something so I trusted the output.  So the bug is simply that the PrintInlining output is misleading.  I'll correct the report but I still want to make the change itself.

tom

> 
> -- Christian
>