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

[Coleen Phillimore]

Cool option to diff.  I didn't know that existed.  In this case, it 
would yield a file that would be mostly impossible to review.  Max's 
change goes a lot further in merging these files, and would be 
unfortunate to go backwards.  This diff would leave a mess in the file 
with a lot of #ifdefs that were easily resolved with register names that 
I wouldn't want to look at.

Coleen

On 3/2/15, 1:37 PM, Dean Long wrote:
> 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
>>>>>
>>>>
>>>
>>
>