RFR: 8013393: Merge templateTable_x86 32 and 64 bit files into one file.

[Christian Thalinger]

That’s quite a large change (I wish we had Crucible or something to leave inline comments).

First, you have to keep the oldest copyright year (but maybe that’s just the diff).

+ #ifdef _LP64
+ void TemplateTable::lconst(int value) {

I’d prefer to have #ifdef’s inside methods and not around them.  There are a bunch; please change all of them.

+ #ifdef _LP64
+   __ movzbl(rdx, Address(rax, rbx, Address::times_1, tags_offset));
+ #else
    __ xorptr(rdx, rdx);
    __ movb(rdx, Address(rax, rbx, Address::times_1, tags_offset));
+ #endif

There is no movzbl on 32-bit?

  void TemplateTable::iaload() {
    transition(itos, itos);
+   // eax: index
    // rdx: array
!   index_check(rdx, rax);  // kills rbx,

That comment should say rax (or maybe the comments should say ax, but that’s for another day).

+   __ load_signed_byte(rax,
+                       Address(rdx, rax,
+                               Address::times_1,
!                               arrayOopDesc::base_offset_in_bytes(T_BYTE)));

That indent is annoying.  Make it one line like before.

-     assert(Bytecodes::java_code(Bytecodes::_fast_iaccess_0) == Bytecodes::_aload_0, "fix bytecode definition");
+            Bytecodes::_aload_0,
+            "fix bytecode definition”);

These as well.

!   __ stop("fast_invokevfinal not used on x86");
!   __ stop("fast_invokevfinal not used on amd64”);

This is wrong.  It should say x86.

!   __ restore_bcp();      // rsi must be correct for exception handler   (was destroyed)
!   __ restore_bcp();      // r13 must be correct for exception handler   (was destroyed)

What are we doing with these?

All in all this is a good change.  Very much appreciated.  Hopefully our testing will show if we made a mistake.

> On Feb 27, 2015, at 9:36 AM, Max Ockner <max.ockner at oracle.com> wrote:
> 
> Hello all,
> Please review this change which involves merging 32 and 64 bit interpreter files.
> 
> Bug ID: 8013393
> Webrev: cr.openjdk.java.net/~coleenp/8013393
> 
> Summary: templateTable_x86_64.cpp and templateTable_x86_32.cpp have been merged into one file*. These files originally shared thousands of lines of duplicate code.
> There were also many nearly identical sections of code which differed only in register usage (example: rsi vs. r13) or in the usage of nearly identical functions (example: movptr vs. movq/movl).
> Given that (1) new bytecodes could be added soon, and (2) these files have been tormenting Coleen for years, this change seems overdue.
> 
> *There are currently two files. The updated templateTable_x86.cpp is copied into both templateTable_x86_32.cpp and templateTable_x86_64.cpp to make it easier to review (so the diff is not the entire file). This will be changed back to one file after review.
> 
> For functions that could be merged, one copy was kept. In some cases, files were merged by factoring out small differences such as register usage (example: rbcp equals r13 or rsi depending on the platform). For functions that could not be merged, the 32 and 64 bit versions are adjacent and are defined conditionally using #ifdef based on the platform.
> 
> There are still improvements to be made, but they are small and can be filed seperately.
> 
> Tested with:
> JPRT
> UTE nsk.jvmti.testlist
> 
> 
> Thanks,
> Max Ockner