RFR(M): 8140594: Various minor code improvements (compiler)

[Tobias Hartmann]

Hi Goetz,

On 09.05.2016 12:30, Lindenmaier, Goetz wrote:
> Hi Tobias,
> 
> sorry for that!  No, it's not just missing include, windows does not 
> implement snprintf before Visual Studio 2015.
> http://stackoverflow.com/questions/2915672/snprintf-and-visual-studio-2010
> 
> As this is only a build-time tool, I would prefer skipping this altogether
> above adding windows specific code:
> http://cr.openjdk.java.net/~goetz/wr16/8140594-covCo/webrev.02/

That's fine with me but unfortunately the build still fails on Windows:

c:/jprt/T/P1/105246.tohartma/s/hotspot/src/share/vm/logging/logTagSet.cpp(112) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of data

Best regards,
Tobias 
> 
> Best regrds,
>  Goetz.
> 
> 
> 
>> -----Original Message-----
>> From: Tobias Hartmann [mailto:tobias.hartmann at oracle.com]
>> Sent: Montag, 9. Mai 2016 11:08
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot compiler
>> <hotspot-compiler-dev at openjdk.java.net>
>> Subject: Re: RFR(M): 8140594: Various minor code improvements (compiler)
>>
>> Hi Goetz,
>>
>> the build failed on Windows with:
>> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(721) :
>> error C3861: 'snprintf': identifier not found
>> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(723) :
>> error C3861: 'snprintf': identifier not found
>> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(728) :
>> error C3861: 'snprintf': identifier not found
>> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(730) :
>> error C3861: 'snprintf': identifier not found
>> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(740) :
>> error C3861: 'snprintf': identifier not found
>> c:/jprt/T/P1/083823.tohartma/s/hotspot/src/share/vm/adlc/dfa.cpp(742) :
>> error C3861: 'snprintf': identifier not found
>>
>> Looks like a missing include.
>>
>> Best regards,
>> Tobias
>>
>> On 09.05.2016 10:40, Tobias Hartmann wrote:
>>> Hi Goetz,
>>>
>>> thanks for the explanation, this looks good to me!
>>>
>>> I'm running the hs-comp PIT now and will push the fix afterwards, if there
>> are no objections.
>>>
>>> Thanks,
>>> Tobias
>>>
>>> On 09.05.2016 10:30, Lindenmaier, Goetz wrote:
>>>> Hi Tobias,
>>>>
>>>>> I can sponsor your fix but since we are close to JDK 9 FC, we are not
>> allowed
>>>>> to push enhancements to hs-comp. Shouldn't this be a "bug" anyway?
>>>> Thanks for reviewing and the offer to sponsor!
>>>> I changed the Jira issue to bug.  Sorry for being that close to FC with my
>> RFR.
>>>>
>>>>>> - size of pointer passed to jio_snprintf()
>>>>> Please also fix the indentation in line 6004.
>>>> Fixed.
>>>>
>>>>>> classLoader.cpp
>>>>>> - jio_snprintf does null termination. But it might return -1 if truncated,
>>>>>>   in this case array access at -1.
>>>>> You can remove "int n;"
>>>> Fixed.
>>>>
>>>>>> generateOopMap.cpp
>>>>>> - Remaining fields not initialized.
>>>>> I would put the loop bodies in a new line.
>>>> Fixed.
>>>>
>>>>>> relocator.cpp
>>>>>> - delta might be -4 ... assert returns.
>>>>> I don't understand this change. If delta is -4, the assert in the baseline
>> version
>>>>> is triggered. With your fix, the assert is triggered as well.
>>>>
>>>> Sorry, that's a bad comment.  I meant that the assert does not
>>>> protect against getting -4 in that line, because coverity knows
>>>> about -XX:SuppressErrorAt.  But that's pointless, as assert goes
>>>> away in product builds anyways.
>>>>
>>>> The assert shall be triggered, therefore I moved it out of the if().
>>>> But the if() now protects against oob with -4 or smaller.
>>>>
>>>> I also rebased the change, and added you as reviewer.  Find the new
>>>> Webrev here:
>>>> http://cr.openjdk.java.net/~goetz/wr16/8140594-covCo/webrev.01/
>>>> And the incremental patch:
>>>> http://cr.openjdk.java.net/~goetz/wr16/8140594-
>> covCo/webrev.01/incremental_diff.patch
>>>>
>>>> Best regards,
>>>>   Goetz.
>>>>