RFR(M): 8233787: Break cycle in vm_version* includes

Daniel D. Daugherty daniel.daugherty at oracle.com
Fri Nov 15 15:08:47 UTC 2019


Adding build-dev at ... since this changeset now touches
make/hotspot/lib/CompileJvm.gmk. The Build team has a standing
request to be included on reviews that touch makefiles...

Dan


On 11/14/19 11:26 AM, Schmidt, Lutz wrote:
> Hi Kim,
>
> that wasn't straightforward. Had to adapt make/hotspot/lib/CompileJvm.gmk. Build settings like HOTSPOT_VERSION_STRING have to flow into the compile step of abstract_vm_version.cpp now. For the details, see my comments below.
>
> Other than that, I hope the new webrev is even closer to your dreams:
>     http://cr.openjdk.java.net/~lucy/webrevs/8233787.02/
>
> Thanks,
> Lutz
>
>
> On 14.11.19, 00:34, "Kim Barrett" <kim.barrett at oracle.com> wrote:
>
>      > On Nov 13, 2019, at 11:42 AM, Schmidt, Lutz <lutz.schmidt at sap.com> wrote:
>      >
>      > Hi Kim,
>      >
>      > there is a new webrev at http://cr.openjdk.java.net/~lucy/webrevs/8233787.01/
>      >
>      > It should be pretty close to what you view as the "right approach". There weren't too many changes relative to 8233787.00. Most files already had #include runtime/vm_version.hpp.
>      
>      This looks much better to me, but many (most?) of the changed
>      #includes need to be moved into sort order.
>
> R: tried my best to fix the sort order. Sorry for not paying attention in the first place.
>      
>      ------------------------------------------------------------------------------
>      src/hotspot/share/runtime/vm_version.cpp
>      
>      Abstract_VM_Version definitions should be moved to abstract_vm_version.cpp.
>      Maybe just rename the file; I think the only thing that would be left
>      for vm_version.cpp would be VM_Version_init().  But maybe that should
>      be left behind in vm_version.cpp?  Though that makes the review messier.
>
> R: Everything moved as you suggested. Doesn't make sense to have Abstract_VM_Version:: methods in vm_version.cpp file.
>      
>      ------------------------------------------------------------------------------
>      src/hotspot/share/runtime/abstract_vm_version.hpp
>      
>      Should #include globalDefinitions.hpp.
>      - uint64_t features()
>      - #define SUPPORTS_NATIVE_CX8
>
> R: Done.
>      
>      Should forward-declare class outsputStream.
>      - print_virtualization_info
>      - print_matching_lines_from_file (I wonder why this is *here*, but not your problem)
>
> R: Done.
>      
>      ------------------------------------------------------------------------------
>      
>      
>      
>




More information about the build-dev mailing list