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