[10] (S) RFR 8191788: add jdk.internal.vm.compiler to --limit-modules if -Djvmci.Compiler=graal is in the command line

[Vladimir Kozlov]

Hi Alan,

On 11/29/17 1:05 AM, Alan Bateman wrote:
> On 28/11/2017 23:51, Vladimir Kozlov wrote:
>> http://cr.openjdk.java.net/~kvn/8191788/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8191788
>>
>> This is redo of JDK-8190975 [1] fix which added jdk.internal.vm.compiler to tests which have --limit-modules option. 
>> Unfortunately tests start failing on SPARC where Graal (jdk.internal.vm.compiler) is not available. JDK-8191653 [2] 
>> reversed 8190975 changes to fix that problem.
>>
>> This fix adds jdk.internal.vm.compiler to --limit-modules list in JVM only when Graal is used: when it is explicitly 
>> specified with -Djvmci.Compiler=graal or in default case and when UseJVMCICompiler is true.
>>
>> I tested the fix with failed tests from JDK-8190975 which are mostly AppCDS tests in test/runtime/appcds/jigsaw and 
>> also test/jdk/java/lang/String/concat/WithSecurityManager.java
>>
>> I think this fix may break upgradeable status of Graal (for Oracle Labs version of Graal).
>> But it should be fine since it is only used with --limit-modules which is not used by Labs.
> If jdk.internal.vm.compiler is not observable on SPARC then shouldn't the tests have `@requires 
> jdk.internal.vm.compiler` and jtreg will skip the test on that platform?

No, these testes (listed above) has nothing to do with Graal. So we can't guard them based on presence of Graal.
That was the problem and that is why original 8190975 fix had to be reversed.

> 
> Just asking as augmenting the value passed to --limit-modules is very strange. It's normal for XX options to augment the 
> set of modules that resolved (+EnableJVMCI implies `--add-modules jdk.internal.vm.ci` for example) but doing this for 
> --limit-modules suggests the VM is doing something to mask an issue with the way that the tests are run.

Just reminder - Graal is upgradeable module [1]. We can't do for it what we do for JVMCI as you pointed.

[1] https://bugs.openjdk.java.net/browse/JDK-8177845

Thanks,
Vladimir

> 
> -Alan