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

[Christian Thalinger]

src/share/vm/c1/c1_GraphBuilder.cpp:

+  void profile_return_type(Value ret, ciMethod* callee, ciMethod* m = NULL, int bci = -1);

+void GraphBuilder::profile_return_type(Value ret, ciMethod* callee, ciMethod* m, int invoke_bci) {
+  assert((m == NULL) == (invoke_bci < 0), "invalid method and invalid bci together");
+  if (m == NULL) {
+    m = method();
+  }
+  if (invoke_bci < 0) {
+    invoke_bci = bci();
+  }

Is there potentially a problem if somehow InvocationEntryBci or InvalidOSREntryBci are passed in:

enum MethodCompilation {
  InvocationEntryBci = -1,     // i.e., not a on-stack replacement compilation
  InvalidOSREntryBci = -2
};

?

+         int bci = invoke_bci;

Why are you having a new variable here?

+           ciTypeEntriesAtCall* args_and_ret = data->is_CallTypeData() ? ((ciCallTypeData*)data)->args_and_ret() : ((ciVirtualCallTypeData*)data)->args_and_ret();

!   const TypeEntriesAtCall* args_and_ret() const { return &_args_and_ret; }

I should've mentioned this in the previous review but can't we make args_and_ret() a virtual method?

src/share/vm/oops/methodData.cpp:

 159 int TypeStackSlotEntries::compute_cell_count(Symbol* signature, int max) {
 160   int args_count = 0;
 161   ResourceMark rm;
 162   SignatureStream ss(signature);
 163   args_count += MIN2(ss.reference_parameter_count(), max);
 164   return args_count * per_arg_cell_count;
 165 }

Why are you using args_count += now?  Before you had:

 164   ResourceMark rm;
 165   SignatureStream ss(inv.signature());
 166   int args_count = MIN2(ss.reference_parameter_count(), max);

! void TypeStackSlotEntries::post_initialize(Symbol* signature, bool has_receiver) {
    ResourceMark rm;
+   int start = 0;
+   ArgumentOffsetComputer aos(signature, _nb_entries-start);
+   aos.total();
+   for (int i = start; i < _nb_entries; i++) {
+     set_stack_slot(i, aos.off_at(i-start) + (has_receiver ? 1 : 0));
+     set_type(i, type_none());
+   }
+ }

I don't see start being changed.  Any reason to have it in a local?  If you want to keep it for clarity make it a const int.

src/share/vm/oops/methodData.hpp:

 753 protected:
 754   const int _nb_entries;

I'd advocate for not abbreviating to ambiguous letters.  Use the word you wanted to use.

On Oct 8, 2013, at 9:22 AM, Roland Westrelin <roland.westrelin at oracle.com> 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.