On Wed, 14 Jun 2023 20:48:36 GMT, Vladimir Ivanov <vliva...@openjdk.org> wrote:
>> Cesar Soares Lucas has updated the pull request with a new target base due >> to a merge or a rebase. The pull request now contains 19 commits: >> >> - Merge branch 'openjdk:master' into rematerialization-of-merges >> - Rome minor refactorings. >> - Merge remote-tracking branch 'origin/master' into >> rematerialization-of-merges >> Catching up with master. >> - Address PR review 6: debug format output & some refactoring. >> - Catching up with master branch. >> >> Merge remote-tracking branch 'origin/master' into >> rematerialization-of-merges >> - Address PR review 6: refactoring around rematerialization & improve test >> cases. >> - Address PR review 5: refactor on rematerialization & add tests. >> - Merge remote-tracking branch 'origin/master' into >> rematerialization-of-merges >> - Address part of PR review 4 & fix a bug setting only_candidate >> - Catching up with master >> >> Merge remote-tracking branch 'origin/master' into >> rematerialization-of-merges >> - ... and 9 more: https://git.openjdk.org/jdk/compare/57b82512...939dcffe > > src/hotspot/share/opto/c2compiler.cpp line 150: > >> 148: if (C.failure_reason_is(retry_no_reduce_allocation_merges())) { >> 149: assert(do_reduce_allocation_merges, "must make progress"); >> 150: do_reduce_allocation_merges = false; > > I consider the check here as a safety net which is intended to provide > graceful degradation in performance if RAM optimization misbehaves for some > reason. But bailing out an optimization is better than bailing out the whole > compilation. I suggest to introduce new diagnostic flag (e.g., > `VerifyReduceAllocationMerges`) and add a guarantee call here which signals > whenever we encounter a problematic case. I'm fine with handling that as a > separate enhancement (it makes sense to dump additional diagnostic info at > the place where such bail outs are triggered ). I created a new [work item](https://bugs.openjdk.org/browse/JDK-8310980) to track this work. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/12897#discussion_r1244208611
