RFR (S): 8026478: -XX:+VerifyAdapterSharing is broken

[Christian Thalinger]

It would be nice to have a comment on this guy:
+   bool    _contains_all_checks;
No need for another webrev if you decide to add the comment.

Looks good.

On Oct 15, 2013, at 11:02 PM, Albert Noll <albert.noll at oracle.com> wrote:

> Forgot to adjust the comment. This version should be fine.
> 
> http://cr.openjdk.java.net/~anoll/8026478/webrev.02/
> 
> Albert
> 
> On 15.10.2013 21:03, Vladimir Kozlov wrote:
>> Albert, 
>> 
>> contains_all_stubs name is incorrect. It should be contains_all_checks. Adapter code does not have stubs in it. 
>> 
>> I think the next check should be: 
>> 
>> bool contains_all_checks = (StubRoutines::code2() != NULL) || !VerifyAdapterCalls; 
>> 
>> because code does not change when VerifyAdapterCalls is false. 
>> Then you don't need to check VerifyAdapterCalls in next condition: 
>> 
>> +       if ((VerifyAdapterCalls || VerifyAdapterSharing) && !entry->contains_all_checks()) { 
>> 
>> And we need to regenerate adapter regardless VerifyAdapterSharing because, as you said, VerifyAdapterCalls potentially reports a false error if check for code2 is not generated. 
>> 
>> So the condition should be simple "if (!entry->contains_all_checks())". 
>> 
>> Could you leave next lines unchanged (use 2 lines for arguments)?: 
>> !       buffer.insts()->initialize_shared_locs((relocInfo*)buffer_locs, 
>> 
>> sizeof(buffer_locs)/sizeof(relocInfo)); 
>>         MacroAssembler _masm(&buffer); 
>> 
>> Cleanup is fine. 
>> 
>> Thanks, 
>> Vladimir 
>> 
>> On 10/15/13 4:14 AM, Albert Noll wrote: 
>>> Hi, 
>>> 
>>> could I have reviews for this small patch? 
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8026478 
>>> webrev: http://cr.openjdk.java.net/~anoll/8026478/webrev.00/ 
>>> <http://cr.openjdk.java.net/%7Eanoll/8026478/webrev.00/> 
>>> 
>>> Problem: This flag is broken. The reason is that adapters 
>>> (gen_i2c_adapter()) that are generated during VM startup do not include 
>>> the range for StubRoutines::code2() if -XX:+VerifyAdapterCalls is 
>>> specified. StubRoutines::code2() is only allocated later during startup; 
>>> consequently, code2() is NULL. As a result, the size of the generated 
>>> adapter as well as the code itself does not mactch. 
>>> 
>>> Solution: Defer check (VerifyAdapterSharing) until VM is initialzied. 
>>> Note that this patch also fixes potential issues with 
>>> -XX:+VerifyAdapterCalls, which is a dignostic flag. The patch also 
>>> removes some unused field of VerifyAdapterSharing. 
>>> 
>>> 
>>> Many thanks in advance, 
>>> Albert 
>>> ** 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20131016/f8193129/attachment-0001.html