RFR (XS): 8144881: Various fixes to linux/sparc

Mikael Vidstedt mikael.vidstedt at oracle.com
Tue Dec 8 18:13:17 UTC 2015


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

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
>> >>>>
>> >>>
>> >>
>> >
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20151208/de499f29/attachment.html>


More information about the ppc-aix-port-dev mailing list