RFR XXS (8185826) HotSpot build failure with GCC 7.1.1

[Ioi Lam]

Yes, I will push it now. Thanks

- Ioi


On 8/3/17 10:17 PM, Yasumasa Suenaga wrote:
> Hi Ioi,
>
> Your change looks good.
> I reviewed (OpenJDK name: ysuenaga).
>
> Could you push this change?
> (I cannot access JPRT)
>
>
> Thanks,
>
> Yasumasa
>
>
> 2017-08-04 14:12 GMT+09:00 David Holmes <david.holmes at oracle.com>:
>> Hi Ioi,
>>
>> On 4/08/2017 2:50 PM, Ioi Lam wrote:
>>> On 8/3/17 7:00 PM, David Holmes wrote:
>>>
>>>> Hi Yasumasa,
>>>>
>>>> On 4/08/2017 10:49 AM, Yasumasa Suenaga wrote:
>>>>> Thanks Kim, David,
>>>>>
>>>>> I filed this issue to JBS and uploaded webrev. Could you review it?
>>>>>
>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8185826/webrev.00/
>>>>
>>>> I'm still at a loss to understand how the current code even
>>>> compiles/links at all with any version of the compiler. ??
>>>>
>>> Hi Yasumasa, thanks for noticing this.
>>>
>>> Sorry, it was my bug. I checked this in as part of JDK-8072061 on Tuesday.
>>>
>>> Apparently older C++ compilers are quite lenient when compiling template
>>> declarations that aren't instantiated. Here's a test case:
>>>
>>> template <typename T> class X {
>>>     static int size() {
>>>       return align_size_up(byte_sizeof());
>>>     }
>>>     static int byte_sizeof() {
>>>       return sizeof(T);
>>>     }
>>> };
>>>
>>> # from gcc 5.4:
>>> $ gcc -c ~/t.cpp && echo $?
>>> 0
>>>
>>> In our case, presumably, gcc will detect the error when an instantiated
>>> Array<T> type access the size(int, int) method.
>>
>> Ah I see.
>>
>>> However, I added this function during my development, but didn't need it
>>> after all. So this function was never referenced, so all of our existing
>>> compilers never had a chance to generate an error for
>>>
>>> I removed this function and HotSpot built with no error.
>>>
>>> $ hg diff
>>> diff -r 731370f39fcd src/share/vm/oops/array.hpp
>>> --- a/src/share/vm/oops/array.hpp    Wed Aug 02 18:06:38 2017 -0700
>>> +++ b/src/share/vm/oops/array.hpp    Thu Aug 03 21:47:41 2017 -0700
>>> @@ -134,10 +134,6 @@
>>>
>>>        return (int)words;
>>>      }
>>> -  static int size(int length, int elm_byte_size) {
>>> -    return align_size_up(byte_sizeof(length, elm_byte_size),
>>> BytesPerWord) / BytesPerWord; // FIXME
>>> -  }
>>> -
>>>      int size() {
>>>        return size(_length);
>>>      }
>>>
>>>
>>> So I think proper fix is to just remove this function.
>>
>> Sounds good to me. Reviewed!
>>
>> Thanks,
>> David
>>
>>> Thanks
>>> - Ioi
>>>
>>>
>>>
>>>
>>>> You removed a "// FIXME" comment - what was that referring to ??
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> I cannot access JPRT. So I need a sponsor.
>>>>>
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>
>>>>>
>>>>> 2017-08-04 9:08 GMT+09:00 Kim Barrett <kim.barrett at oracle.com>:
>>>>>>> On Aug 3, 2017, at 7:12 PM, David Holmes <david.holmes at oracle.com>
>>>>>>> wrote:
>>>>>>> I can't even see a definition of align_size_up - where is it coming
>>>>>>> from ???
>>>>>>
>>>>>> It was recently removed.
>>>>>>
>>>>>> 8178499: Remove _ptr_ and _size_ infixes from align functions
>>>>>>