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

[Christian Thalinger]

On Tue, 2010-05-18 at 12:25 -0700, Tom Rodriguez wrote:
> 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.

That makes sense.  Then the changes look good.  -- Christian