RFR (XS): 8144881: Various fixes to linux/sparc
Mikael Vidstedt
mikael.vidstedt at oracle.com
Tue Dec 8 20:18:11 UTC 2015
Volker, Dmitry - thanks for the reviews!
Cheers,
Mikael
On 2015-12-08 10:21, Volker Simonis wrote:
> On Tue, Dec 8, 2015 at 7:13 PM, Mikael Vidstedt
> <mikael.vidstedt at oracle.com> wrote:
>> On Dec 8, 2015, at 01:42, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
>>
>> Hi guys,
>>
>> On Tue, Dec 8, 2015 at 8:58 AM, Volker Simonis <volker.simonis at gmail.com>
>> wrote:
>>> Hi Mikael,
>>>
>>> as David mentioned, your change should be synchronized with Thomas'
>>> change, who is currently cleaning up os::print_siginfo() (see:
>>>
>>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2015-December/017012.html).
>>>
>> Thanks for considering me! But I think this does not conflict with my
>> change. Even if it does I can merge Mikaels changes in.
>>
>>
>> Thanks, much appreciated.
>>
>>
>>> Regarding your change, I can't understand the reason for including
>>> code/codeCache.hpp into all os_cpu/vm/os_<os>_<cpu>.cpp files. These
>>> files compiled fine until now, so why unnecessarily adding another
>>> dependency?
>>>
>> That would be my question too.
>>
>>
>> I can go either way on this one, really no strong opinions.
>>
>> The fact is that the files in question *do* make use of CodeCache without
>> including the codeCache.hpp. I can only assume that in the other cases
>> codeCache.hpp gets included implicitly - either through precompiled.hpp or
>> through some other header file. I found at least one case where it wasn't
>> included at all, and that resulted in a build error.
>>
>> For correctness and consistency reasons it could be argued that
>> codeCache.hpp really should be included where it's logic is used, but as I
>> said I really don't have a strong opinion. At all.
>>
> OK, you're right. I wasn't clear to me before but CodeCache is indeed
> used in the corresponding files.
>
> In that case your change is good - please leave it as it is.
>
> Regards,
> Volker
>
>> Cheers,
>> Mikael
>>
>>
>>
>> Kind Regards, Thomas
>>
>>> Regards,
>>> Volker
>>>
>>>
>>> On Tue, Dec 8, 2015 at 6:21 AM, David Holmes <david.holmes at oracle.com>
>>> wrote:
>>>> Mikael,
>>>>
>>>> I'll take a look at this asap but it likely conflicts with other changes
>>>> that Thomas Stufe has out for review wrt the signal handling code.
>>>>
>>>> I'm somewhat suspicious of the sudden need to change linux code just
>>>> because
>>>> of the sparc port, however I shall reserve judgement for now :)
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>
>>>>
>>>> On 8/12/2015 9:16 AM, Mikael Vidstedt wrote:
>>>>>
>>>>> Good control question! Since these are build problems, and since
>>>>> building on other platforms works today, making corresponding changes
>>>>> to
>>>>> files for other platforms is not strictly necessary, but for
>>>>> consistency
>>>>> it's of course reasonable to do so anyway. Here's an updated webrev
>>>>> doing exactly that:
>>>>>
>>>>> http://cr.openjdk.java.net/~mikael/webrevs/8144881/webrev.01/webrev/
>>>>>
>>>>> Changes since webrev.00:
>>>>>
>>>>> * Copyright year updated (*grumble*)
>>>>> * Changes to print sigflags in check_signal_handler in os_linux.cpp
>>>>> were
>>>>> also applied to the other os_*.cpp files (where applicable)
>>>>> * code/codeCache.hpp included in all os_cpu/vm/os_<os>_<cpu>.cpp files
>>>>> (where applicable)
>>>>> * Added return statement to other JVM_handle_*_signal (where
>>>>> applicable)
>>>>> - these functions could *really* use some additional
>>>>> cleanup/unification...
>>>>> * Also: I noticed that the rest of the code in os_linux_sparc.cpp uses
>>>>> "sigcontext", as opposed to "struct sigcontext", so I dropped the
>>>>> "struct"
>>>>>
>>>>> Note: I'll have to build this again to make sure it still builds
>>>>> correctly across all the platforms, so consider this an optimistic
>>>>> webrev+review.
>>>>>
>>>>> Cheers,
>>>>> Mikael
>>>>>
>>>>>
>>>>> On 2015-12-07 14:19, Dmitry Samersoff wrote:
>>>>>> Mikael,
>>>>>>
>>>>>> The changes looks good for me, but
>>>>>>
>>>>>> Most of changes don't look as sparc-specific.
>>>>>>
>>>>>> Do you need to update other platforms as well?
>>>>>>
>>>>>> -Dmitry
>>>>>>
>>>>>> On 2015-12-08 00:53, Mikael Vidstedt wrote:
>>>>>>> Please review this small change which fixes a handful of places which
>>>>>>> currently lead to build failures on linux/sparc. The changes are all
>>>>>>> relatively straightforward and come in a few different flavors:
>>>>>>>
>>>>>>> * Incorrect format specifiers
>>>>>>> * "reaches end of function returning value"
>>>>>>> * wrong type name (sigcontext_t -> struct sigcontext)
>>>>>>> * wrong variable name (addr -> pc)
>>>>>>>
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8144881
>>>>>>> Webrev:
>>>>>>> http://cr.openjdk.java.net/~mikael/webrevs/8144881/webrev.00/webrev/
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Mikael
>>>>>>>
>>
More information about the ppc-aix-port-dev
mailing list