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

[Dean Long]

Hi Coleen.  Let's say the _32 file contains

32
x
y
z
int

and the _64 file contains

64
x
y
z
long

Then "diff -DLP64 _32 _64" will give:

#ifndef LP64
32
#else /* LP64 */
64
#endif /* LP64 */
x
y
z
#ifndef LP64
int
#else /* LP64 */
long
#endif /* LP64 */

In other words, an automatic merge of the two files, with the 
differences in #ifdefs.

thanks,

dl

On 3/2/2015 7:01 AM, Coleen Phillimore wrote:
>
> Hi Dean,
> I'm not sure how to do this diff that you're suggesting.  I don't 
> think an intermediate changeset makes sense either. The bytecodes are 
> pretty well tested with our normal testing so increasing this to more 
> than one change, just increases testing and putback overhead.
>
> I've looked at this several times and it's not very difficult to tell 
> what's done and whether it's correct.  The diffs do not span a lot of 
> code (at most 5 lines at a time), so there are no new whole functions 
> to read that are new code.
>
> I'm still really don't understand what kind of diff you're requesting 
> that would make it easier.  Can you provide an example?
>
> thanks,
> Coleen
>
> On 2/27/15, 6:13 PM, Dean Long wrote:
>> Hi Coleen.  I believe I understand what Max did, and that it was not 
>> an automated merge,
>> which to me says that properly reviewing the change requires looking 
>> at all 2851
>> changed lines and mentally deciding if the result is equivalent to 
>> the original.
>>
>> What I was suggesting was essentially two things.  First, an aid to 
>> the reviewer,
>> and second, a way to reduce risk.  In the end it's up to the 
>> reviewers.  If they
>> prefer to see the changes in a single diff, they can compare a 
>> single-file
>> automated merge with the single-file result of Max's merge.  It 
>> doesn't matter
>> if the reviewer does it or Max does it as a convenience. Regarding risk,
>> if the reviewers aren't comfortable deciding that all 2851 are correct,
>> the changes could be staged into several pieces, and the first piece 
>> could
>> be an automated merge that would be cleaned up later, on the assumption
>> that an automated merge could be considered trivially correct and 
>> therefore
>> not risky.  I hope that clears up my somewhat terse comments below.
>>
>> dl
>>
>> On 2/27/2015 12:06 PM, Coleen Phillimore wrote:
>>>
>>> Dean,  This merge wasn't automatic in any way, so I don't think what 
>>> you're proposing makes sense.  I think if you look at the webrev 
>>> you'll see what Max has done.
>>> Thanks,
>>> Coleen
>>>
>>> On 2/27/15, 1:39 PM, Dean Long wrote:
>>>> Hi Max.  You could do an automatic merge using something like "diff 
>>>> -w -D LP64 templateTable_x86_32.cpp and templateTable_x86_64.cpp", 
>>>> then compare
>>>> your improvements to that.  That way all the changes would show up in
>>>> a single diff.  Or you could even check in the automatic merge 
>>>> first, to reduce
>>>> risk, then break up the improvements into smaller pieces.
>>>>
>>>> dl
>>>>
>>>> On 2/27/2015 9:36 AM, Max Ockner 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
>>>>
>>>
>>
>