RFR (XL): 7172640: C2: instrinsic implementations in LibraryCallKit should use argument() instead of pop()

[Christian Thalinger]

I made a couple of smaller changes based on a verbal review from John:

- removed (void) casts for null_check* calls
- renamed null_check_receiver_as_before_call to null_check_receiver_before_call
- removed for_parse argument to GraphKit::compute_stack_effects

webrev is updated and I'm going to push that change now.  Thanks.

-- Chris

On Nov 21, 2012, at 5:02 PM, Christian Thalinger <christian.thalinger at oracle.com> wrote:

> 
> On Nov 21, 2012, at 3:56 PM, John Rose <john.r.rose at oracle.com> wrote:
> 
>> On Nov 20, 2012, at 6:47 PM, Christian Thalinger wrote:
>> 
>>> http://cr.openjdk.java.net/~twisti/7172640
>> 
>> That's a lovely cleanup.  The "to push or not to push" stuff around null checks is swept to the background.  The reexecute_sp cut point clears things up nicely.
>> 
>> I especially like that GraphKit::_sp is now private.  The helper methods like get_field_at_bci are tasteful.
>> 
>> The huge swaths of code deletion are great.  The number of deleted lines is twice as big as the number of inserted lines!
>> 
>> A few comments:
>> 
>> I think the call to "kit.push_result()" should be guarded (internally or externally) by "kit.stopped()".  The guard "result != NULL" is a little off, though it probably doesn't hurt.
> 
> Yes.  Done.
> 
>> 
>> I don't think do_null_check_receiver pulls its weight, though it's OK.  Are you sure the argument(0) value can never be top?  That would make the assert fail.  If you delete the assert, then there's nothing interesting in that method.
> 
> Hmm.  No, I'm not sure is can never be top.  I added do_null_check_receiver to emphasize that we are checking a receiver.  Maybe we don't need it?
> 
>> 
>> (It seems that null_check_common now wants to be called "do_null_check_common".  Actually, the whole "do_" prefix thing is bogus noise.  Maybe we can undo "do_" now for those functions?)
> 
> Good idea.  I removed all the do_ prefixes.  Additionally I added default parameters because we almost always pass in T_OBJECT.
> 
> webrev is updated.
> 
> -- Chris
> 
>> 
>> — John
>