RFR(L) 8026054: New type profiling points: type of return values at calls

[Vladimir Kozlov]

On 10/9/13 9:18 AM, Roland Westrelin wrote:
> Hi Vladimir,
>
> Thanks for reviewing this.
>
>> Sorry, but I will not be able to debug this in 3 month :( TypeStackSlotEntries is already complex enough and you including return type cell into it (by using TypeEntriesAtCall) to avoid additional cell when no object return.
>>
>> Can we always have one separate cell for return value for CallTypeData and VirtualCallTypeData when arguments are profiling. I would prefer to have that cell always instead of complicated code which tries to calculate its presence. From what I understand when you profile only one argument TypeProfileArgsLimit=1 you will use additional cell for cells count even if no return will be recorded (if return type is not profiled cell count is not added in this case). It looks like the space usage will be the same as when you always generate cell for return type for this case (for 2 and more arguments you will get extra one but it is 3+1). I think it will simplify the code because you don't need to change TypeStackSlotEntries.
>>
>> So can you separate TypeStackSlotEntries and ReturnTypeEntry and don't use TypeEntriesAtCall?:
>>
>>   class VirtualCallTypeData : public VirtualCallData {
>>   private:
>> !   TypeStackSlotEntries _args;
>> !   ReturnTypeEntry       _ret;
>>
>> Yes, CallTypeData and VirtualCallTypeData will have more additional duplicated code. You can put some common code into TypeEntries (for example, compute_cell_count() is static).
>>
>> And TypeStackSlotEntries should be ArgTypeEntries because stack slot is just data used to record arguments types.
>
> ArgTypeEntries seems to imply it's only used for arguments. In the next change, it's used for parameters. That's why I named it TypeStackSlotEntries.

I see, then may be CallTypeEntries. Again, we are not profiling stack slots.

>
> What about this:
> http://cr.openjdk.java.net/~roland/8026054/webrev.01/
> ?

It is better. About header_cell_count(), previous version was different. 
As I understand you always need cell_count cell to separate case when 
you have only 1 argument vs no profilable arguments but have ret type. 
Right? So my suggestion to have always ret_type cell will not help.

Another question. You execute arguments profiling in assembler only if 
tag is CallTypeData or VirtualCallTypeData. But you don't do such check 
for return_type profiling. Why?

About assembler. Why you need subl(tmp, 1) (there is no comment)? And 
why it is not done during stack_slot cell initialization?

About C1 code for ret profiling. I see only new code in 
c1_LIRGenerator.cpp which calls profile_arg_type() and no new assembler. 
Is it enough?

thanks,
Vladimir

>
> I kept TypeEntriesAtCall only for the static methods but otherwise refactored as you suggested, I think.
> This also includes Chris' feedback.
>
> Roland.
>
>>
>> Thanks,
>> Vladimir
>>
>> On 10/8/13 9:22 AM, Roland Westrelin wrote:
>>> http://cr.openjdk.java.net/~roland/8026054/webrev.00/
>>>
>>> Follow up to 8023657: New type profiling points: arguments to call
>>>
>>> 8023657 introduced TypeStackSlotEntries to keep track of types of arguments. This change adds a couple more similar classes: TypeEntriesAtCall and ReturnTypeEntry which is maybe the confusing part of the change. ReturnTypeEntry keeps track of types of return values, TypeStackSlotEntries of types of arguments. In the next type profiling change, TypeStackSlotEntries will be used for parameter type profiling. TypeEntriesAtCall is ReturnTypeEntry + TypeStackSlotEntries to do the profiling at invokes. The number of cells needed at invokes is kept in TypeEntriesAtCall (to count cells for arguments and return value) and some methods in TypeStackSlotEntries were moved to TypeEntriesAtCall as a consequence.
>>>
>>> The output of PrintMethodData is:
>>>
>>> 17 invokevirtual 3 <TestProfiling.m2(LTestProfiling$C;Ljava/lang/Object;Ljava/lang/Object;)LTestProfiling$C;>
>>>    16  bci: 17   VirtualCallData     count(0) entries(1)
>>>                                      'TestProfiling'(4744 1.00)
>>>                  argument types      0: stack(1) 'TestProfiling$C'
>>>                                      1: stack(2) 'TestProfiling$B'
>>>                  return type         'TestProfiling$C'
>>>
>>> Roland.
>>>
>