[15] RFR(M) 8237497: vmStructs_jvmci.cpp does not check that the correct field type is specified

Vladimir Kozlov vladimir.kozlov at oracle.com
Fri Mar 20 19:32:32 UTC 2020


Thank you, Ioi, for looking on changes.

On 3/20/20 11:56 AM, Ioi Lam wrote:
> Hi Vladimir,
> 
> Why do you need jvmci_vmStructs_init()? Is it possible to call JVMCIVMStructs::init() directly?

For that I have to #include vmStructs_jvmci.hpp which will pull other headers as well. Which would increase parsing time 
of this C++ code even for product build. And I followed what we do for vmStruct - it calls vmStructs_init().

> 
> Also, I think you can change the declaration as
> 
> class JVMCIVMStructs {
>     void init() NOT_DEBUG_RETURN;
> }
> 
> So the caller can avoid using DEBUG_ONLY().

I don't like hiding that a called code is executed only in debug more.
And why do we need to keep empty methods in product builds?

It takes a lot of time to compile HotSpot VM.

> 
> The rest looks good to me.

Thank you,
Vladimir K

> 
> Thanks
> - Ioi
> 
> On 3/20/20 10:01 AM, Vladimir Kozlov wrote:
>> http://cr.openjdk.java.net/~kvn/8237497/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8237497
>>
>> JVMCI missed checks for declarations in vmStructs_jvmci.cpp. I copied check logic from vmStruct to fix that. It will 
>> be called during JVMCI initialization in debug build (as for vmStruct). I can't call it from vmStructs_init() because 
>> vmStruct code is guarded by INCLUDE_VM_STRUCTS variable which can be switched off with configure option 
>> --with-jvm-features=-vm-structs. On other hand JVMCIVMStruct have to be initialized always when JVMCI is used - it is 
>> how Graal accesses HotSpot's internal data.
>>
>> The fix found several (4) cases of incorrect types in JVMCI declarations. 2 of them require update Graal code as well 
>> which I will upstream later.
>>
>> I also did cleanup in both, vmStructs_jvmci and vmStructs.
>> In vmStructs_jvmci I removed macros which are not used (we can add them back if they needed).
>> In vmStructs I moved all check code under #ifdef ASSERT because it is called only in debug build.
>> I removed VMStructs::test() declaration which we forgot to remove when its body was removed in JDK-8171090.
>> I removed duplicated lines which existed from "day one" (before Mercurial).
>>
>> I did local build and testing with and without --with-jvm-features=-vm-structs to make sure everything (inluding 
>> Graal) worked. I ran tier1 (which runs HotSpot gtests), hs-tier2, hs-tier3-graal.
>>
>> Thanks,
>> Vladimir
> 


More information about the hotspot-compiler-dev mailing list