RFR: 8317697: refactor-encapsulate x86 VM_Version::CpuidInfo [v2]
David Holmes
dholmes at openjdk.org
Tue Oct 24 10:05:30 UTC 2023
On Mon, 23 Oct 2023 06:27:15 GMT, Jan Kratochvil <jkratochvil at openjdk.org> wrote:
>> src/hotspot/cpu/x86/vm_version_x86.hpp line 527:
>>
>>> 525: };
>>> 526:
>>> 527: class CpuidInfo : public _CpuidInfo {
>>
>> Why not just declare the original `CpuidInfo` as a class instead of extending the struct ???
>
> This way the members are always zero-initialized. Which simplifies existing code
>
> -VM_Version::CpuidInfo VM_Version::_cpuid_info = { 0, };
> +VM_Version::CpuidInfo VM_Version::_cpuid_info;
>
> And makes it more foolproof - therefore also fixing an existing bug (I haven't been aware of so far) in my code in the [CRaC](https://openjdk.org/projects/crac/) branch.
>
> Although the ` = { 0, }` initialization above was not needed as it is a static member anyway.
>
> Or should I remove the zero-initialization?
Sorry I'm not familiar enough with C++ initialization rules for structs versus classes here. IIUC:
- the original struct instance was explicitly zero-initialized.
- If we just made it a class there would not be any zero initialization
- If the class extends the struct then we (somehow) regain zero initialization
??
Maybe methods on a struct weren't so bad after all. I'd like to hear other views on this code change. (Just to be clear 2 reviews are needed for hotspot code changes anyway.)
Thanks
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16093#discussion_r1369915053
More information about the hotspot-dev
mailing list