[9] RFR: 8044538: assert(which != imm_operand) failed: instruction is not a movq reg, imm64

[Tobias Hartmann]

Thank you, Chris.

Best,
Tobias

On 10.06.2014 17:38, Christian Thalinger wrote:
> Looks good.
>
> On Jun 10, 2014, at 8:04 AM, Tobias Hartmann 
> <tobias.hartmann at oracle.com <mailto:tobias.hartmann at oracle.com>> wrote:
>
>> Hi Chris,
>>
>>> Your @run command uses the wrong class name:
>>>
>>>    28  * @run main/othervm -XX:+IgnoreUnrecognizedVMOptions -Xcomp -XX:+PrintRelocations Test8044538
>>>    35 public class TestPrintRelocations {
>>
>> Thanks, I missed that while renaming. Should have tested it..
>>
>> New webrev: http://cr.openjdk.java.net/~thartmann/8044538/webrev.04/
>>
>> Best,
>> Tobias
>>
>>>
>>> On Jun 9, 2014, at 11:10 PM, Tobias Hartmann 
>>> <tobias.hartmann at oracle.com <mailto:tobias.hartmann at oracle.com>> wrote:
>>>
>>>>>>
>>>>>> Hi Chris,
>>>>>>
>>>>>>>> Hi Vladimir,
>>>>>>>>
>>>>>>>> thanks for the review.
>>>>>>>>
>>>>>>>>> Looks good. PrintRelocations flag is used very rare, that is 
>>>>>>>>> why it rotted. Please, add jreg test (use 
>>>>>>>>> -XX:+IgnoreUnrecognizedVMOptions).
>>>>>>>> I added a jtreg test to test/compiler/8044538/ that uses -Xcomp 
>>>>>>>> and -XX:+PrintRecolations. It fails without my changes.
>>>>>>> We are trying to not use bug id directories anymore.  Please 
>>>>>>> move the test to a more meaningful directory with a meaningful name.
>>>>>> Okay, makes sense. I moved the test to 
>>>>>> test/compiler/PrintRelocations/TestPrintRelocations.java
>>>>> Thanks but that’s too fine grained.  I doubt there will be any 
>>>>> other PrintRelocations tests.  So maybe test/compiler/relocations/ 
>>>>> makes more sense.
>>>>
>>>> Okay, I moved it to test/compiler/relocations.
>>>>
>>>> New webrev:http://cr.openjdk.java.net/~thartmann/8044538/webrev.03/ 
>>>> <http://cr.openjdk.java.net/%7Ethartmann/8044538/webrev.03/>
>>>>
>>>> Thanks,
>>>> Tobias
>>>>
>>>>>
>>>>>> New webrev: 
>>>>>> http://cr.openjdk.java.net/~thartmann/8044538/webrev.02/ 
>>>>>> <http://cr.openjdk.java.net/%7Ethartmann/8044538/webrev.02/>
>>>>>>
>>>>>> Thanks,
>>>>>> Tobias
>>>>>>
>>>>>>>> New webrev: 
>>>>>>>> http://cr.openjdk.java.net/~thartmann/8044538/webrev.01/ 
>>>>>>>> <http://cr.openjdk.java.net/%7Ethartmann/8044538/webrev.01/>
>>>>>>>>
>>>>>>>>> Since this debug flag almost never used backporting the fix 
>>>>>>>>> only into 8u may be enough.
>>>>>>>> Okay.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Tobias
>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Vladimir
>>>>>>>>>
>>>>>>>>> On 6/4/14 1:30 AM, Tobias Hartmann wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> please review the following patch for bug 8044538.
>>>>>>>>>>
>>>>>>>>>> *Problem*
>>>>>>>>>> Executing a debug build of HotSpot with the flags 
>>>>>>>>>> -XX:+PrintRelocations
>>>>>>>>>> -Xcomp hits a ShouldNotReachHere() or an assert in
>>>>>>>>>> Assembler::locate_operand(..) stating that the instruction 
>>>>>>>>>> for which we
>>>>>>>>>> try to find the operand is not valid.
>>>>>>>>>>
>>>>>>>>>> The problem occurs while printing the relocation entries for a C2
>>>>>>>>>> compiled function. The C2 compiler adds internal_word_type 
>>>>>>>>>> relocations
>>>>>>>>>> for the jump table entries in the constant section of a 
>>>>>>>>>> method (see
>>>>>>>>>> Compile::ConstantTable::fill_jump_table(...)). These 
>>>>>>>>>> relocations are
>>>>>>>>>> processed by RelocIterator::print_current(...) and
>>>>>>>>>> internal_word_Relocation::target().
>>>>>>>>>> Relocation::pd_get_address_from_code() then tries to retrieve the
>>>>>>>>>> address from an instruction but fails because the relocation 
>>>>>>>>>> points into
>>>>>>>>>> the constant section only containing the target address.
>>>>>>>>>>
>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8044538
>>>>>>>>>>
>>>>>>>>>> *Solution*
>>>>>>>>>> The implementation of internal_word_Relocation::target() is 
>>>>>>>>>> changed to
>>>>>>>>>> check if the relocation points into the constant section and 
>>>>>>>>>> if so
>>>>>>>>>> directly returns the target address instead of trying to 
>>>>>>>>>> retrieve it
>>>>>>>>>> from an instruction. The same is already done in
>>>>>>>>>> internal_word_Relocation::fix_relocation_after_move(..).
>>>>>>>>>>
>>>>>>>>>> Webrev: 
>>>>>>>>>> http://cr.openjdk.java.net/~thartmann/8044538/webrev.00/ 
>>>>>>>>>> <http://cr.openjdk.java.net/%7Ethartmann/8044538/webrev.00/>
>>>>>>>>>>
>>>>>>>>>> *Tests*
>>>>>>>>>> Failing configuration, JPRT
>>>>>>>>>>
>>>>>>>>>> Apparently this did not show up for any of our tests. Do we 
>>>>>>>>>> need an
>>>>>>>>>> additional test for this?
>>>>>>>>>> Since it already fails for JDK 7 and 8. Should we backport 
>>>>>>>>>> the patch?
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Tobias
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140610/68aec6bd/attachment-0001.html>