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

[Max Ockner]

Hello all,
I have made all of the changes recommended by Chris and John.  I realize 
there are plenty of further cleanups and optimizations that I could 
make, but I think this is nearing a good stopping point for now. We can 
always clean it up more later. What do you think?

new webrev:   cr.openjdk.java.net/~coleenp/8013393
tested with nsk.jvmti.testlist

On 3/2/2015 4:45 PM, John Rose wrote:
> On Mar 2, 2015, at 11:46 AM, Christian Thalinger <christian.thalinger at oracle.com> wrote:
>> 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.
I have moved the #ifdef's to the inside of the function declarations. 
Some of the #ifdefs could be moved far enough in that it made sense to 
replace them with the LP64_ONLY() or NOT_LP64() tools. The remaining 
#ifdef's are there because some large sections of code could not be 
merged, and needed to be kept together.

> I agree Christian's comments, and especially with this request.
>
> A few more points on small matters:
>
> In our source base, C++ comments usually begin with two slashes and a space (less than 10% exceptions).
>    +//Global Register Names
>    +//Address Computation: local variables
>    (etc.)

Fixed.

> Let's use the term "amd64" only where required; x86_64 is the standard now.
>     // No amd64 specific initialization

Fixed.

> More on #ifdefs:  I think there are too many; Chris has pointed out the main problem.
>
> It's a judgement call, but here's another example:
>
> +#ifdef _LP64
> +        Register rtmp = r8;
> +        Register rthread = r15_thread;
> +#else
> +        Register rtmp = rsi;
> +        Register rthread = rcx;
> +        __ get_thread(rcx);
> +        __ save_bcp();
> +#endif // _LP64
>
> could just as well be:
>
> +        Register rtmp = LP64_ONLY(r8) NOT_LP64(rsi);
> +        Register rthread = LP64_ONLY(r15_thread) NOT_LP64(rcx);
> +        LP64_ONLY(__ get_thread(rcx));
> +        LP64_ONLY(__ save_bcp());
>
> Advantages:  Fewer lines, less duplication, less chance of merge problems.
> Possible objection:  Ugly; but only in a different way from the other ugly.

Fixed, but there are some #ifdef's left over. There are some pretty 
large code blocks that are conditionally defined. I don't think it makes 
sense to enclose every conditionally defined line of code in an 
LP64_ONLY or NOT_LP64. At best you save 3 lines of code, but I feel that 
a those 3 lines would be less distracting than having 25 LP64_ONLY's in 
a row.
>
> However, I admit that is a marginal example.
>
> — John

Thanks,
Max Ockner