RFR (S) 8239492: [x86] Turn MacroAssembler::verify_oop into macro recording file and line

Per Liden per.liden at oracle.com
Mon Feb 24 10:14:24 UTC 2020



On 2/24/20 10:17 AM, Reingruber, Richard wrote:
> 
>>>> I was a little confused as now the verify_oop() calls in TemplateTable get dispatched directely to
>>> MacroAssembler, but that's what InterpreterMacroAssembler::verify_oop() used to do there anyway,
>>> because all the call sites got the default argument state = atos.
> 
>> Yes. Sometimes interp_verify_oop is called with the actual "state", but otherwise it is called with
>> atos. We could, in principle, introduce two separate macro definitions to simulate that default
>> argument, but I'd prefer not to go overly crazy with macros.
> 
> Sure.
> 
>>> I assume you deliberately did the renaming in zVerify.cpp instead of undefining verify_oop as in
>>> shenandoahVerifier.cpp. That's good for me too.

I would prefer having the two static functions being renamed to 
z_verify_oops and z_verify_possibly_weak_oop, like you did in webrev.02.

thanks,
Per

> 
>> Ah, hm. Actually, maybe we should indeed do it Shenandoah-way by undef-ing the macro in the
>> problematic compilation unit.
> 
> I'll leave it up to you.
> 
> Thanks,
> Richard.
> 
> -----Original Message-----
> From: Aleksey Shipilev <shade at redhat.com>
> Sent: Freitag, 21. Februar 2020 18:57
> To: Reingruber, Richard <richard.reingruber at sap.com>; hotspot-dev at openjdk.java.net
> Subject: Re: RFR (S) 8239492: [x86] Turn MacroAssembler::verify_oop into macro recording file and line
> 
> On 2/21/20 6:32 PM, Reingruber, Richard wrote:
>>> Suggested fix:
>>>    https://cr.openjdk.java.net/~shade/8239492/webrev.02/
>>
>> The RFE is worth the (small) effort and the proposed change looks good to me.
> 
> Thanks.
> 
>>> I was a little confused as now the verify_oop() calls in TemplateTable get dispatched directely to
>> MacroAssembler, but that's what InterpreterMacroAssembler::verify_oop() used to do there anyway,
>> because all the call sites got the default argument state = atos.
> 
> Yes. Sometimes interp_verify_oop is called with the actual "state", but otherwise it is called with
> atos. We could, in principle, introduce two separate macro definitions to simulate that default
> argument, but I'd prefer not to go overly crazy with macros.
> 
>> I assume you deliberately did the renaming in zVerify.cpp instead of undefining verify_oop as in
>> shenandoahVerifier.cpp. That's good for me too.
> 
> Ah, hm. Actually, maybe we should indeed do it Shenandoah-way by undef-ing the macro in the
> problematic compilation unit.
> 
>> You should update the years in the copyright headers.
> 
> Updated.
> 
> New webrev:
>    https://cr.openjdk.java.net/~shade/8239492/webrev.03/
> 


More information about the hotspot-dev mailing list