JDK-8160748: inconsistent types for ideal_reg

[Yasumasa Suenaga]

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
>>>>
>>>>