RFR: 8309697: [TESTBUG] Remove "@requires vm.flagless" from jtreg vectorization tests [v2]

[Emanuel Peter]

On Fri, 4 Aug 2023 20:10:19 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Hi @vnkozlov ,
>> 
>> Thanks for your reply. But it still has problems.
>> 
>>> About your change to allow -Xbatch. Let me clarify, if you exclude -Xcomp mode (which I agree with) by checking UseInterpreter flag for true, then a method could be always executed in Interpeter to get reference result (even with -XX:CompileThreshold=100) by calling method once first (we do that in other tests).
>> 
>>> You don't need to call WB.lockCompilation() if you exclude -Xcomp mode. There will be no compilation requests for called method when you call the method first time because compilation threshold will not be reached - it is guarantee that method will be executed in Interpreter. And you have the assert to verify that.
>> 
>> These tests are a bit different because we test loops. If the loop iteration count reaches some threshold, the loop will be *OSR compiled* even test method is called only once. I just did an experiment according to your suggestion. After removing `WB.lockCompilation()` and updating loop iteration count to 100,000, I got assertion failure that tells me the test method is NOT running in interpreter.
>> 
>> 
>> STDERR:
>> java.lang.AssertionError
>> 	at compiler.vectorization.runner.VectorizationTestRunner.runTestOnMethod(VectorizationTestRunner.java:131)
>> 	at compiler.vectorization.runner.VectorizationTestRunner.run(VectorizationTestRunner.java:73)
>> 	at compiler.vectorization.runner.VectorizationTestRunner.main(VectorizationTestRunner.java:215)
>> 	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
>> 	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
>> 	at com.sun.javatest.regtest.agent.MainWrapper$MainTask.run(MainWrapper.java:138)
>> 	at java.base/java.lang.Thread.run(Thread.java:1570)
>> 
>> 
>> A solution to this may be adding one more check of `CICompileOSR` is OFF if we still want to use interpreted execution for the reference result.
>> 
>> Now the question is, which verification approach do you think is better? "C2 vs. interpreted" or "C2 vs. C1"?
>
>> A solution to this may be adding one more check of `CICompileOSR` is OFF if we still want to use interpreted execution for the reference result.
> 
> I would suggest to use `WB.setBooleanVMFlag("CICompileOSR", false);`. But it is debug flag which can be set only in debug VM.  There are may be other product flags you can temporary set to avoid compilation without locking.
> 
>> 
>> Now the question is, which verification approach do you think is better? "C2 vs. interpreted" or "C2 vs. C1"?
> 
> We usually use Interpreter as gold standard.

@vnkozlov @TobiHartmann we should re-run testing from our side.

@pfustc Why do you only test correctness (compare results) in some conditions? Is there not a risk that we miss doing it in some cases we should do it, just because we get the conditions slightly wrong?

Just FYI: we should integrate this whole correctness of results testing into the IR framework. I filed [JDK-8310533](https://bugs.openjdk.org/browse/JDK-8310533). That would make it easier to use for new tests. It could also be used for any test, not just the ones located in `test/hotspot/jtreg/compiler/vectorization`.

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

PR Comment: https://git.openjdk.org/jdk/pull/15011#issuecomment-1670043666