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

[Christian Thalinger]

Thank you, Vladimir.  -- Chris

On Nov 21, 2012, at 3:01 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:

> Good.
> 
> Vladimir
> 
> On 11/21/12 14:41, Christian Thalinger wrote:
>> 
>> On Nov 20, 2012, at 8:13 PM, Vladimir Kozlov<vladimir.kozlov at oracle.com>  wrote:
>> 
>>> Could you factor out the assert in callnode.hpp?
>> 
>> Done.
>> 
>>> 
>>> graphKit.cpp:
>>> 
>>> +   // If the bytecode needs to be reexecuted we need a stack pointer with
>>> +   // the arguments on the stack.
>>> ----
>>> +   // If the bytecode needs to be reexecuted we need to put
>>> +   // the arguments back on the stack.
>>> 
>>> +   // NOTE: set_bci (called from sync_jvms) might reset the reexecute bit to
>>> +   // undefined if the bci is different.  This is okay for Parse but is a
>>> +   // problem for LibraryCallKit where we explicitly set the reexecute bit.
>>> ----
>>> +   // NOTE: set_bci (called from sync_jvms) will reset the reexecute bit to
>>> +   // undefined if the bci is different.  This is normal for Parse but it
>>> +   // should not happen for LibraryCallKit because only one bci is processed.
>> 
>> Done.  webrev updated.
>> 
>> -- Chris
>> 
>>> Thanks,
>>> Vladimir K.
>>> 
>>> On Nov 20, 2012, at 6:47 PM, Christian Thalinger wrote:
>>> 
>>>> http://cr.openjdk.java.net/~twisti/7172640
>>>> 
>>>> 7172640: C2: instrinsic implementations in LibraryCallKit should use argument() instead of pop()
>>>> Reviewed-by:
>>>> 
>>>> Most intrinsics in LibraryCallKit re-push the arguments that have been
>>>> popped off before the call to just pop them again as arguments.  It
>>>> would be much cleaner and more correct to use argument() accessors
>>>> instead.
>>>> 
>>>> Another problem is that it is very tricky to find the places where the
>>>> stack pointer needs to be adjusted to the correct value for
>>>> re-execution (uncommon traps).  This adjustment is a plain SP bump and
>>>> decrease after the call.  In itself very error prone and it could be
>>>> handled in a much more elegant way.
>>>> 
>>>> src/share/vm/ci/ciMethod.cpp
>>>> src/share/vm/ci/ciMethod.hpp
>>>> src/share/vm/ci/ciSignature.hpp
>>>> src/share/vm/interpreter/bytecodes.hpp
>>>> src/share/vm/opto/callGenerator.cpp
>>>> src/share/vm/opto/callnode.hpp
>>>> src/share/vm/opto/doCall.cpp
>>>> src/share/vm/opto/graphKit.cpp
>>>> src/share/vm/opto/graphKit.hpp
>>>> src/share/vm/opto/library_call.cpp
>>>> src/share/vm/opto/parse1.cpp
>>>> src/share/vm/opto/parse2.cpp
>>>> src/share/vm/opto/parse3.cpp
>>>> src/share/vm/opto/parseHelper.cpp
>>>> src/share/vm/opto/type.hpp
>>>> 
>>> 
>>