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

[Vladimir Kozlov]

On Wed, 16 Dec 2020 14:09:20 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

Code changes seems fine.

test/hotspot/jtreg/compiler/codecache/TestStressCodeBuffers.java line 30:

> 28:  *          which can fail more often with -XX:+StressCodeBuffers. Perform some more sanity flag testing.
> 29:  * @run main/othervm -Xcomp -XX:-TieredCompilation -XX:+StressCodeBuffers compiler.codecache.TestStressCodeBuffers
> 30:  * @run main/othervm -Xcomp -XX:+StressCodeBuffers compiler.codecache.TestStressCodeBuffers

StressCodeBuffers is debug flag. You need to add `-XX:+IgnoreUnrecognizedVMOptions` flag or `@requires vm.debug == true`

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

Changes requested by kvn (Reviewer).

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