RFR: 8257513: C2: assert((constant_addr - _masm.code()->consts()->start()) == con.offset()) [v2]

[Vladimir Kozlov]

On Thu, 17 Dec 2020 13:36:14 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Running the testcase with `-XX:+StressCodeBuffers` initializes the code buffer with a small size:
>> https://github.com/openjdk/jdk/blob/0c8cc2cde47bed3d5c3edc203869068a0676812b/src/hotspot/share/opto/output.cpp#L1339-L1340
>> This leads to more frequent expansions in `ConstantTable::emit()` when calling the `_masm.*_constant()` methods. In the `T_VOID` case, we additionally fill the jump-table with dummy words:
>> https://github.com/openjdk/jdk/blob/0c8cc2cde47bed3d5c3edc203869068a0676812b/src/hotspot/share/opto/constantTable.cpp#L146-L150
>> 
>> When there is an expansion during this loop, then `constant_addr` is still pointing to an outdated address before expanding the code buffer and relocating the data. This lets the assertion fail when subtracting the new address `_masm.code()->consts()->start()` after the expansion.
>> 
>> I extended this assertion for `T_VOID` to take an expansion into account. 
>> 
>> There is a second issue that is currently not handled correctly. The `_masm.*_constant()` methods could return `NULL` when we failed to allocate new space in an expansion which, however, rarely happens. But when running with `-XX:+StressCodeBuffers`, `NULL` is much more likely to be returned because we intentionally free a blob from time to time:
>> https://github.com/openjdk/jdk/blob/0c8cc2cde47bed3d5c3edc203869068a0676812b/src/hotspot/share/asm/codeBuffer.cpp#L846-L854
>> 
>> I therefore suggest to also change the associated assertions in `ConstantTable::emit()` into bailout code to handle this case (as also done in the `LIR_Assembler` using the `AbstractAssembler::*_constant()` methods).
>> 
>> Thanks,
>> Christian
>
> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add @vm.debug == true to test

Good.

-------------

Marked as reviewed by kvn (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1803