RFR 8144534: Refactor templateInterpreter and templateInterpreterGenerator functions

John Rose john.r.rose at oracle.com
Fri Dec 4 01:13:17 UTC 2015

On Dec 3, 2015, at 3:51 PM, Coleen Phillimore <coleen.phillimore at oracle.com> wrote:
> The most interesting part of this change is in the x86 code:
> http://cr.openjdk.java.net/~coleenp/8144534.02/src/cpu/x86/vm/templateInterpreterGenerator_x86.cpp.udiff.html <http://cr.openjdk.java.net/~coleenp/8144534.02/src/cpu/x86/vm/templateInterpreterGenerator_x86.cpp.udiff.html>
> This is where the 32 and 64 bit template interpreter code is merged.

This is a magnificent case of CDE (Code Deletion Engineering).

About 1/3 of the textual lines in each merged file are unique:

$ diff -U9999 templateInterpreter_x86_{32,64}.cpp > templateInterpreter_x86_MERGED.cpp
$ wc templateInterpreter_x86_{32,64,MERGED}.cpp
    1916    7955   71138 templateInterpreter_x86_32.cpp
    1866    7348   67194 templateInterpreter_x86_64.cpp
    2610   12638  104592 templateInterpreter_x86_MERGED.cpp

Any many of the unique lines differ from their counterparts only trivially.

I read over the changes and found what I hoped to find.
A few of the multi-line #ifdefs could be compressed even more,
but that can be done later if we want, as we touch the code.

Cleanups like this make our system more maintainable.
They also reduce the cost of future changes, like value types.

Thanks for doing this.  You can count me as a reviewer.

— John

P.S. One request:  Please prefer putting the semicolons outside of the macro calls,
even when they are logically inside.  It makes it easier for auto-indenting tools
like Emacs to DTRT.  (They treat semicolon at the end of a line as a signal to
indent normally on the next line, but a paren at the end of the line is a signal
to indent extra on the next line, on the assumption that it is a continuation line.)

The majority convention appears to be putting the semicolon outside the paren:
	grep '_ONLY(.*);' $(hg loc -I src) | wc
     667    3514   65119
	grep '_ONLY(.*;)' $(hg loc -I src) | wc
     216     868   19884
	grep 'NOT_[A-Z0-9]*(.*);' $(hg loc -I src) | wc
     855    4098   82619
	grep 'NOT_[A-Z0-9]*(.*;)' $(hg loc -I src) | wc
     296    1320   27929

More information about the hotspot-dev mailing list