Request for reviews (S): 7090259: Fix hotspot sources to build with old compilers

[Tom Rodriguez]

On Sep 14, 2011, at 12:34 PM, Bengt Rutisson wrote:

> 
> Vladimir and Tom,
> 
> Thanks for providing the background information.
> 
> Just to verify that I got it right: there seems to be a bug related to templates in the vs2003 compiler, but we are not sure exactly what template patterns that will trigger the bug.

I suspect it was templating a function which wasn't part of a class.  I would guess that this would work:

class MethodComparator {
  template <C> int compare(C* a, C* b);
};

The bug was pretty clearly a problem which how method_comparator was being generated and not how it was being used.

> 
> Any suggestions for how to do testing if I do another change that involves templates?

Running it through JPRT with -release jdk6 is the only thing I can think of.  We should probably do this more regularly than we do.

tom

> 
> Thanks,
> Bengt
> 
> 
> On 2011-09-14 19:05, Tom Rodriguez wrote:
>> On Sep 14, 2011, at 8:59 AM, Vladimir Kozlov wrote:
>> 
>>> On 9/14/11 12:36 AM, Bengt Rutisson wrote:
>>>> Hi again,
>>>> 
>>>>> Also, how long will we be supporting vs2003? I was under the impression that we had switched to vs2010 even for JDK6.
>>>>> Clearly I was wrong. Does that mean that we will keep vs2003 for the duration of JDK6 and thus implicitly have that as
>>>>> a build requirement for JDK7 as long as we are using the express model?
>>>> I realized that I did know about vs2003 being the compiler for JDK6. Sorry for the noise.
>>>> 
>>>> I am still interested in understanding what templates to avoid with vs2003.
>>> This is what Tom found analyzing crush:
>> I'm still confused by it.  It appears to think the arguments are in eax and edi which doesn't match any calling convention I can find.  I guess it's just a bug.
>> 
>> tom
>> 
>>> On 9/13/11 9:52 AM, Tom Rodriguez wrote:
>>>> I'm looking at it in the debugger and it appears to be crashing in the method_comparator in the new QuickSort code. It sort of looks like the passed in pointers are small integers instead of pointers but the array itself looks ok. maybe it's an optimizer bug?
>>>> 
>>>> Actually I think it's a disagreement about the calling convention.  The caller pushes the arguments:
>>>> 
>>>> 080e5409 50              push    eax
>>>> 080e540a 51              push    ecx
>>>> 080e540b ff5510          call    dword ptr [ebp+10h]
>>>> 080e540e 83c408          add     esp,8
>>>> 
>>>> but the callee thinks they are in registers:
>>>> 
>>>> jvm!method_comparator<oopDesc *>:
>>>> 080e78a0 8b4808          mov     ecx,dword ptr [eax+8]
>>>> 080e78a3 0fb74928        movzx   ecx,word ptr [ecx+28h]
>>>> 080e78a7 53              push    ebx
>>>> 080e78a8 8b5f08          mov     ebx,dword ptr [edi+8
>>>> 
>>>> There seems to be a bit of mixing of calling convention going on in the binary, which seems very odd.
>>>> 
>>>> tom
>>> I think it is because one of template classes (C comparator) for QuickSort::sort is method type which is also template.
>>> 
>>> Vladimir
>>> 
>>> 
>>>> Thanks
>>>> Bengt
>>>> 
>>>>> 
>>>>> Bengt
>>>>> 
>>>>> On 2011-09-14 01:31, Vladimir Kozlov wrote:
>>>>>> http://cr.openjdk.java.net/~kvn/7090259/webrev
>>>>>> 
>>>>>> 7090259: Fix hotspot sources to build with old compilers
>>>>>> 
>>>>>> Fixed warnings which prevent building VM with old compilers. Used non-template method_comparator() in sort_methods()
>>>>>> since vs2003 generates incorrect optimized code for it. Added -xwe compilation flag. Tested with 5.7, 5.8 and 5.10
>>>>>> Sun C++.
>>>>>> 
>>>>>> I will remove g1ErgoVerbose.hpp changes and push to hotspot-gc since Tony fixed it already there.
>>>>>> 
>>>>>> Thanks,
>>>>>> Vladimir
>