RFR: 8271471: [IR Framework] Rare occurrence of "<!-- safepoint while printing -->" in PrintIdeal/PrintOptoAssembly can let tests fail [v4]

[Tobias Hartmann]

On Tue, 10 Aug 2021 09:33:54 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> A test VM used by the IR framework sometimes prints `` in the middle of emitting a `PrintIdeal` or `PrintOptoAssembly` output which could lead to IR matching failures:
>> https://github.com/openjdk/jdk/blob/489e5fd12a37a45f4f5ea64b05f85c6f99f70811/src/hotspot/share/utilities/ostream.cpp#L918-L927
>> 
>> I thought about just bailing out of IR matching if this string is found after a failure but this issue also affects internal framework tests (I observed one case locally where this happened in the test `TestIRMatching`, letting it fail).
>> 
>> Handling `` makes things more complicated for the IR framework tests. I'm not sure about the value of printing this message in the first place. But if nobody objects, I suggest to either remove it or at least guard it with `Verbose`, for example. I went with the latter solution in this PR.
>> 
>> Thanks,
>> Christian
>
> Christian Hagedorn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Renaming and change handling for TestScenarios

Looks good to me. I'm just wondering if we need all that `notAllBailedOut`/`noneBailedOut` complexity for handling scenarios. Given that safepoints during printing are rare, couldn't we simply skip verification of the exception message if one scenario hit a bailout?

test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/TestScenarios.java line 51:

> 49:                 Asserts.fail("Should have thrown an exception");
> 50:             }
> 51: 

Newline can be removed

test/hotspot/jtreg/testlibrary_tests/ir_framework/tests/Utils.java line 52:

> 50:     }
> 51: 
> 52:     private static void shouldHaveThrownException(String testVMOutput) {

Now this method is only used by `shouldHaveThrownException()`. I would remove it.

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

Marked as reviewed by thartmann (Reviewer).

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