Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8254129: IR Test Framework to support regex-based matching on the IR in JTreg compiler tests #3508

Closed
wants to merge 29 commits into from
Closed
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
a563fb3
8254129: IR Test Framework to support regex-based matching on the IR …
chhagedorn Apr 13, 2021
aa005f3
Add Javadoc files
chhagedorn Apr 14, 2021
5ada49e
Fix whitespaces
chhagedorn Apr 15, 2021
4ec7f0e
Fix whitespaces + minor fix
chhagedorn Apr 15, 2021
cc6e285
Remove accidentally added print statement
chhagedorn Apr 15, 2021
e284341
Fix comment
chhagedorn Apr 15, 2021
7ed789d
Adjust whitelist
chhagedorn Apr 15, 2021
b3f5811
Apply review comments in code from Igor I. and Vladimir K.
chhagedorn Apr 19, 2021
6e5a954
Fix typos and grammar, code format (JohnTortugo), fix exception throw…
chhagedorn Apr 20, 2021
5451ad2
Replace "\n" by System.lineSeparator()
chhagedorn Apr 20, 2021
433006c
Improve error format output by changing some new lines
chhagedorn Apr 20, 2021
72eece6
Remove Javadocs and README.html, update README.md to reference java f…
chhagedorn Apr 20, 2021
0720a33
Review comments by Tobias: Formatting fields, spacing, README.md typo…
chhagedorn Apr 23, 2021
0a3cc3f
Apply review comments: Added new Compiler annotation class for @DontC…
chhagedorn Apr 27, 2021
90a0064
Fix XCOMP cases from old framework and turn it into new debug flag -D…
chhagedorn Apr 27, 2021
5890c4a
Apply review comments: Extract Test classes into own files, extract F…
chhagedorn May 3, 2021
8f56dd6
Rename TestFrameworkPrepareFlags -> FlagVM and rename TestFrameworkEx…
chhagedorn May 3, 2021
b08235c
Minor improvements, comment fixes, and test fixes
chhagedorn May 3, 2021
d6c72ec
Remove TestFramework: both runWithScenarios, both runWithHelperClasse…
chhagedorn May 3, 2021
fd25de7
Move framework to test/hotspot/jtreg/compiler/lib and tests to test/h…
chhagedorn May 4, 2021
a9f0429
Fix package names and fixing internal tests, examples and README file…
chhagedorn May 4, 2021
4424e01
Splitting classes into subpackages and updating README accordingly, f…
chhagedorn May 4, 2021
85a5921
Merge branch 'master' into JDK-8254129
chhagedorn May 31, 2021
c35c658
Fix Compiler and CompLevel ANY and fix tests after merge
chhagedorn May 31, 2021
7a316de
Add more whitelisted flags
chhagedorn Jun 2, 2021
df7576f
Fix failing internal tests on Windows and add missing flag descriptio…
chhagedorn Jun 3, 2021
2ba1338
Move tests and examples #1
chhagedorn Jun 4, 2021
e6b2f60
Move tests and examples #2
chhagedorn Jun 4, 2021
7b77370
Update test and example package names, README files and fix some test…
chhagedorn Jun 4, 2021
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
@@ -107,7 +107,6 @@
Arrays.asList(

This comment has been minimized.

Loading
@iignatev

iignatev Apr 16, 2021
Member Outdated

it doesn't seem to be a comprehensive list of flags that don't affect IR verification work, e.g. different GCs. I think it might be easy to go with the blacklist instead, and possibly warning people if there are any flags in test.*.opts.

This comment has been minimized.

Loading
@chhagedorn

chhagedorn Apr 19, 2021
Author Member Outdated

Maybe first some general background about having a whitelist/blacklist in the first place. When a user writes an @IR rule, we do not want him to think about any possible VM flag combination (which he is not specifically using in his test, for example with addFlags) that could break his @IR rule and then requiring him to restrict the @IR rule with IR::applyIfXX() for these flags.

I agree that the whitelist is probably not complete and could be improved (but I think we should not whitelist different GCs as these affect the barrier code of the IR). About whitelist vs. blacklist, I had discussions about it with @TobiHartmann. We eventually decided to use a whitelist which seemed easier to maintain and is probably a lot shorter than a blacklist. In addition, if a new flag is introduced or removed, we most likely do not need to worry about updating the whitelist as the (currently) whitelisted flags will most likely remain unchanged. This first whitelist version is also just a best guess solution. We might miss on some other flags that will not affect the IR but we have the confidence that we do not run into surprising failures due to forgetting to blacklist a particular flag. But this is open for discussion of course. It would be interesting to hear how people think about it.

This comment has been minimized.

Loading
@iignatev

iignatev Apr 19, 2021
Member Outdated

I'll need to think about it a bit more, but my first impression after reading your comment is that we don't actually want to allow any flags, so we might better off just using @requies vm.flagless or reusing the underlying code to check that we don't have any flags that can "significantly" change behavior.

This comment has been minimized.

Loading
@chhagedorn

chhagedorn Apr 20, 2021
Author Member Outdated

Our idea was to only restrict the IR verification done after the test VM terminates with the whitelist (or blacklist). But the framework should still run each test with any flag combination. This showed to be quite useful in Valhalla to find bugs.

// The following substrings are part of more than one VM flag
"RAM",
"G1",
"Heap",
"Trace",
"Print",
@@ -116,15 +115,12 @@
"UseNewCode",
// The following substrings are only part of one VM flag (=exact match)
"CreateCoredumpOnCrash",
"IgnoreUnrecognizedVMOptions",
"UnlockDiagnosticVMOptions",
"UnlockExperimentalVMOptions",
"BackgroundCompilation",
"Xbatch",
"TieredCompilation",
"UseSerialGC",
"UseParallelGC",
"UseG1GC",
"UseZGC",
"UseShenandoahGC"
"TieredCompilation"
)
);