JDK-8160748: inconsistent types for ideal_reg

[Vladimir Kozlov]

Thank you very much, Yasumasa

Changes look good. We will push them when JDK 10 repository is opened (I don't know when that happens).

Regards,
Vladimir

On 7/11/16 7:06 AM, Yasumasa Suenaga wrote:
> Hi Vladimir, Kim,
>
>>> I do not understand where you have concern.
>>> So please tell me where I should change in .ad files.
>>
>> Here you need to change 'int' to 'uint' for opcode parameter and return type:
>
> I clarified all points which we should fix with C++11 scoped enum:
>
>   http://cr.openjdk.java.net/~ysuenaga/JDK-8160748/webrev.01/
>
> I hope this webrev helps you to work for this enhancement.
>
>
> Thanks,
>
> Yasumasa
>
>
> On 2016/07/06 23:52, Vladimir Kozlov wrote:
>> Hi,
>>
>> On 7/6/16 5:30 AM, Yasumasa Suenaga wrote:
>>> Hi Vladimir,
>>>
>>> On 2016/07/06 7:23, Vladimir Kozlov wrote:
>>>> Good cleanup. We should have done it long ago.
>>>> But you missed .ad files where Matcher methods are defined.
>>>
>>> I can build HotSpot with this patch without changes for .ad files.
>>> I think it implies we do not need to change for .ad files.
>>>
>>> I changed adlc source (output_c.cpp). Is it not enough?
>>
>> Not enough.
>>
>>>
>>> I do not understand where you have concern.
>>> So please tell me where I should change in .ad files.
>>
>> Here you need to change 'int' to 'uint' for opcode parameter and return type:
>>
>> const bool Matcher::match_rule_supported(int opcode) {
>>
>> const bool Matcher::match_rule_supported_vector(int opcode, int vlen) {
>>
>> const int Matcher::vector_ideal_reg(int size) {
>>
>> const int Matcher::vector_shift_count_ideal_reg(int size) {
>>
>>>
>>>
>>>> And you need FC Extension Request since JDK-8160748 is Enhancement.
>>>
>>> Kim says this issue does not fix probably in JDK 9 at all.
>>> I guess he think it is fixed in JDK 10 or later completely.
>>>
>>> However, I think this type mismatch should be fixed ASAP.
>>> If it is accepted, I will add jdk9-fc-request label to JBS.
>>>
>>> Can I do it?
>>
>> No, as Kim said it is not a problem with jdk 9 which uses c++98 to build. Lets do it in jdk 10. Changes are too big.
>> Wait when jdk 10 repository is open.
>>
>>>
>>>
>>>> After the push may need to file the backport for JDK 8u to keep sources in sync (a lot of C2 files changed).
>>>
>>> I do not have any role for jdk8u.
>>
>> We don't need to do that if we skip to jdk 10.
>>
>> Thanks,
>> Vladimir
>>
>>> So I need a sponsor.
>>>
>>>
>>> Thanks,
>>>
>>> Yasumasa
>>>
>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 7/3/16 7:54 AM, Yasumasa Suenaga wrote:
>>>>> Hi all,
>>>>>
>>>>> We've worked for JDK-8160353 to resolve narrowing conversion error at
>>>>> GCC 6.
>>>>> ------------
>>>>> /home/ysuenaga/OpenJDK/hs/hotspot/src/share/vm/opto/type.cpp:101:1:
>>>>> error: narrowing conversion of '(uint)Node::NotAMachineReg' from 'uint
>>>>> {aka unsigned int}' to 'int' inside { } [-Werror=narrowing]
>>>>>  };
>>>>>  ^
>>>>> /home/ysuenaga/OpenJDK/hs/hotspot/src/share/vm/opto/type.cpp:101:1:
>>>>> error: narrowing conversion of '(uint)Node::NotAMachineReg' from 'uint
>>>>> {aka unsigned int}' to 'int' inside { } [-Werror=narrowing]
>>>>> /home/ysuenaga/OpenJDK/hs/hotspot/src/share/vm/opto/type.cpp:101:1:
>>>>> error: narrowing conversion of '(uint)Node::NotAMachineReg' from 'uint
>>>>> {aka unsigned int}' to 'int' inside { } [-Werror=narrowing]
>>>>> cc1plus: all warnings being treated as errors
>>>>> ------------
>>>>>
>>>>> Opcodes is defined as enum.
>>>>> However, a part of code (e.g. NotAMachineReg) is defined as uint.
>>>>>
>>>>> I think Opcodes related code should consist uint type.
>>>>>
>>>>>   http://cr.openjdk.java.net/~ysuenaga/JDK-8160748/webrev.00/
>>>>>
>>>>> I've uploaded webrev for this issue.
>>>>> I fixed Opcodes type to uint at first, and fixed many errors with GCC 6.
>>>>>
>>>>> I want to discuss about this.
>>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Yasumasa
>>>>>
>>>>>