RFR: 8325372: Shenandoah: SIGSEGV crash in unnecessary_acquire due to LoadStore split through phi [v2]

[Roman Kennke]

On Tue, 20 Feb 2024 15:17:10 GMT, Roland Westrelin <roland at openjdk.org> wrote:

>> After shenandoah barrier expansion, a shenandoah specific pass looks
>> for heap stable checks that are back to back:
>> 
>> 
>> if (heap_stable) {
>>   // fast path 1
>> } else {
>>   // slow path 1
>> }
>> if (heap_stable) {
>>   // fast path 2
>> } else {
>>   // slow path 2
>> }
>> 
>> 
>> and fuse them:
>> 
>> 
>> if (heap_stable) {
>>   // fast path 1
>>   // fast path 2
>> } else {
>>   // slow path 1
>>   // slow path 2
>> }
>> 
>> 
>> In the case of the failure, a `GetAndSetP` (or `GetAndSetN`) node is
>> between the 2 heap_stable checks. The fusion of the 2 tests is
>> implemented by taking advantage of the split if c2 optimization. But
>> split if doesn't support having a `GetAndSet` node at the region where
>> split if happens (that can only happen with shenandoah late barrier
>> expansion). That causes the `GetAndSet` node to lose its `SCMemProj`
>> which can then result in the `GetAndSet` being entirely removed.
>> 
>> The fix I propose is to not perform the heap_stable fusion in this
>> particular case.
>
> Roland Westrelin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update test/hotspot/jtreg/gc/shenandoah/compiler/TestUnsafeLoadStoreMergedHeapStableTests.java
>   
>   Co-authored-by: Aleksey Shipilëv <shipilev at amazon.de>

Looks good to me, thank you!
Roman

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

Marked as reviewed by rkennke (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17926#pullrequestreview-1895619215