RFR: JDK-8322854: Incorrect rematerialization of scalar replaced objects in C2

[Cesar Soares Lucas]

On Mon, 29 Jan 2024 09:40:03 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Current implementation of `PhaseMacroExpand::value_from_mem` returns `return _igvn.zerocon(ft);` when it hits a sentinel while searching for a memory operation on a given slice. One of the sentinels is the memory input of the allocate node origin of the memory slice. Therefore, `value_from_mem` may return `zeroconf(ft)` if `sfpt_mem` is the same memory edge used by the Allocate node origin of the memory slice being traversed.
>> 
>> The scalar replacement implementation uses `value_from_mem` during creation of metadata describing object scalar replaced (see `PhaseMacroExpand::create_scalarized_object_description`). The `create_scalarized_object_description` method is also used as part of RAM optimization implementation. The RAM optimization targets Phi nodes and therefore a memory graph loop created by a _memory phi_ node is possible to seen as part of the transformation. See image below:
>> 
>> <img src="https://github.com/openjdk/jdk/assets/2249648/288681b7-4461-41ea-8dab-bbaebac4727f" height="500" />
>> 
>> This pattern doesn't show up when scalarizing objects that don't participate in allocation merges. 
>> 
>> To fix the issue I changed the code in `value_from_mem` to instead of using the _input_ memory edge of the Allocate as a stop condition, it will now use the projection memory edge of the Allocate.
>> 
>> Tested locally on windows, mac and linux x86_64 with JTREG tier1-3 and didn't observe any regression.
>
> I executed some quick testing and I see failures with
> - compiler/gcbarriers/TestArrayCopyWithLargeObjectAlignment.java	
> - compiler/loopstripmining/TestLoadOnBackedgeWithPrec.java
> - compiler/membars/TestMemBarAcquire.java
> 
> 
>   668  Phi  === 49 669 670  [[ 667 818 817 816 ]]  #rawptr:BotPTR !jvms: TestLoadOnBackedgeWithPrec::j @ bci:159 (line 55)
>   668  Phi  === 49 669 670  [[ 667 818 817 816 ]]  #rawptr:BotPTR !jvms: TestLoadOnBackedgeWithPrec::j @ bci:159 (line 55)
>    41  safePoint_poll_tls  === 47 0 819 0 0 42 1421 820 1423 616 0 46 45 44  |1645  [[ 43 39 ]]  !jvms: TestLoadOnBackedgeWithPrec::j @ bci:190 (line 46)
> #
> # A fatal error has been detected by the Java Runtime Environment:
> #
> #  Internal Error (/opt/mach5/mesos/work_dir/slaves/0db9c48f-6638-40d0-9a4b-bd9cc7533eb8-S9853/frameworks/1735e8a2-a1db-478c-8104-60c8b0af87dd-0196/executors/2cd88c15-c79b-46eb-a505-4fbf2c345f82/runs/27421909-81f2-433f-811e-a8e03bb02478/workspace/open/src/hotspot/share/opto/buildOopMap.cpp:365), pid=978899, tid=978928
> #  assert(false) failed: there should be an oop in OopMap instead of a live raw oop at safepoint
> 
> 
> With `-ea -esa -XX:CompileThreshold=100 -XX:+UnlockExperimentalVMOptions -server -XX:-TieredCompilation` on AArch64.

@TobiHartmann - did any test fail in your latest run?

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

PR Comment: https://git.openjdk.org/jdk/pull/17562#issuecomment-1936672718