RFR (M) 8146410: Interpreter functions are declared and defined in the wrong files

[Lindenmaier, Goetz]

> -----Original Message-----
> From: Coleen Phillimore [mailto:coleen.phillimore at oracle.com]
> Sent: Dienstag, 5. Januar 2016 21:01
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-dev
> developers <hotspot-dev at openjdk.java.net>
> Subject: Re: RFR (M) 8146410: Interpreter functions are declared and defined
> in the wrong files
> 
> 
> 
> On 1/5/16 2:51 PM, Lindenmaier, Goetz wrote:
> > Hi Coleen,
> >
> > I think they belong together because can_be_compiled() just returns
> > false for functions that have a math entry which is generated by
> > generate_math_entry().  So if you add a new math_entry, you
> > also have to fix can_be_compiled.  We wired this dependency into
> > math_entry_available(kind), so that both functions are guaranteed
> > to behave similar.
> 
> I'm getting confused by this.  It seems like math_entry_available should
> be in templateInterpreterGenerator near generate_math_entry.
> 
> The reason I don't want can_be_compiled (which is misleading) is because
> all the other platforms have it too and I don't want them in the wrong
> files.
> 
> what do you think?
What I mean is can_be_compiled() should be moved to class 
templateInterpreterGenerator and placed near generate_math_entry()
on all platforms.
The similarity is the same on all of them.

But I must admit in compilationPolicy.cpp it reads better if the 
AbstractInterpreter is asked. Also, method_kind is member
of AbstractInterpreter.

But method_kind is also completely over-engeneered, as it's
only used in can_be_compiled, and there it's only used
for math entries...

Maybe a change of it's own is needed...

Best regards,
  Goetz.







> 
> Coleen
> >
> > The name of the function is kind of misleading to me.
> >
> > But I won't insist...
> >
> > Best regards,
> >    Goetz.
> >
> >
> >
> >> -----Original Message-----
> >> From: Coleen Phillimore [mailto:coleen.phillimore at oracle.com]
> >> Sent: Tuesday, January 05, 2016 7:17 PM
> >> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-dev
> >> developers <hotspot-dev at openjdk.java.net>
> >> Subject: Re: RFR (M) 8146410: Interpreter functions are declared and
> defined
> >> in the wrong files
> >>
> >>
> >>
> >> On 1/5/16 2:19 AM, Lindenmaier, Goetz wrote:
> >>> Hi Coleen,
> >>>
> >>> Could you please also move
> >>> AbstractInterpreter::can_be_compiled(methodHandle m)
> >>> to above
> >>>
> >>
> TemplateInterpreterGenerator::generate_math_entry(AbstractInterpreter::
> >> MethodKind kind)?
> >>> I think these belong together.
> >> Really?  They're in different classes (hence the motivation for the
> >> change) and generate_math_entry doesn't call can_be_compiled.
> >> CompilationPolicy does but it calls it from
> >> AbstractInterpreter::can_be_compiled() so I really can't move this
> >> function into TemplateInterpreterGenerator.  It doesn't make sense to
> me.
> >>
> >>> We seem to have different BIND macros on ppc. The '__' is also in the
> >>> macro. Could you please fix this small issue? It breaks the ppc build.
> >>>
> >>> diff -r 743aa331fc90
> >> src/cpu/ppc/vm/templateInterpreterGenerator_ppc.cpp
> >>> --- a/src/cpu/ppc/vm/templateInterpreterGenerator_ppc.cpp	Tue
> >> Jan 05 07:47:21 2016 +0100
> >>> +++ b/src/cpu/ppc/vm/templateInterpreterGenerator_ppc.cpp	Tue
> >> Jan 05 08:06:12 2016 +0100
> >>> @@ -416,7 +416,7 @@
> >>>      default: ShouldNotReachHere();
> >>>      }
> >>>
> >>> -  __ BIND(done);
> >>> +  BIND(done);
> >>>      __ blr();
> >>>
> >> Fixed.  Thanks!!
> >>>      return entry;
> >>>
> >>> Besides this, the change looks good.
> >> Thanks,
> >> Coleen
> >>
> >>> Thanks and best regards,
> >>>     Goetz.
> >>>
> >>> PS: is it possible to share your Copyright script?
> >>>
> >>>> -----Original Message-----
> >>>> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net]
> On
> >>>> Behalf Of Coleen Phillimore
> >>>> Sent: Monday, January 04, 2016 11:43 PM
> >>>> To: hotspot-dev developers <hotspot-dev at openjdk.java.net>
> >>>> Subject: RFR (M) 8146410: Interpreter functions are declared and
> defined
> >> in
> >>>> the wrong files
> >>>>
> >>>> Summary: Moved functions to the correct files.
> >>>>
> >>>> See bug for more details.
> >>>>
> >>>> I basically did an hg mv templateInterpreter_<cpu>.cpp
> >>>> abstractInterpreter_<cpu>.cpp and moved the interpreter_<cpu>.cpp
> >>>> functions there.
> >>>>
> >>>> Also moved generate_slow_signature_handler to
> >>>> TemplateInterpreterGenerator/CppInterpreterGenerator because it's
> >> not
> >>>> shared.
> >>>>
> >>>> open webrev at http://cr.openjdk.java.net/~coleenp/8146410/
> >>>> bug link https://bugs.openjdk.java.net/browse/JDK-8146410
> >>>>
> >>>> Tested with JPRT on Oracle supported platforms and built zero on linux
> >>>> x86.  Also fixed change that broke zero in stack_zero.inline.hpp.  I
> >>>> think this should work on PPC and AARCH64, but please let me know.
> >>>>
> >>>> One question for AARCH64 platform in file:
> >>>>
> >>>>
> >>
> http://cr.openjdk.java.net/~coleenp/8146410/src/cpu/aarch64/vm/templat
> >>>> eInterpreterGenerator_aarch64.cpp.udiff.html
> >>>>
> >>>> thanks,
> >>>> Coleen