Build change looks ok.
/Erik
On 2019-11-15 07:08, Daniel D. Daugherty wrote:
Adding build-dev@... 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.barr...@oracle.com> wrote:
> On Nov 13, 2019, at 11:42 AM, Schmidt, Lutz
<lutz.schm...@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.
------------------------------------------------------------------------------
