-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8370489: Some compiler tests miss the @key randomness #28463
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
base: master
Are you sure you want to change the base?
Conversation
|
👋 Welcome back snatarajan! A progress list of the required criteria for merging this PR into |
|
@sarannat This change is no longer ready for integration - check the PR body for details. |
Webrevs
|
chhagedorn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and trivial, thanks for cleaning these up!
| package compiler.vectorization; | ||
| import java.util.Random; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| package compiler.vectorization; | |
| import java.util.Random; | |
| package compiler.vectorization; | |
| import java.util.Random; |
dafedafe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the cleanup @sarannat. Looks good to me.
| @@ -30,9 +30,10 @@ | |||
| import jdk.test.lib.Asserts; | |||
| import jdk.test.lib.Utils; | |||
|
|
|||
| /* | |||
| /** | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need javadoc style comments for JTreg? (we don't seem to be too consistent in our tests)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is true. There is no need for javadoc style comments (see The JDK Test Framework: Tag Language Specification). I have fixed this file.
You are also right about we not being consistent with the tests. I have left the rest of the test (in this changeset) with javadoc style as it is. Do let me know if I should change them ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I would not worry too much about comment style. Well actually: are we sure that both get executed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the following tests in this changeset that started with /** (javadoc style) with /** and /*. Both the styles resulted in the same results.
test/hotspot/jtreg/compiler/intrinsics/float16/TestFloat16MaxMinSpecialValues.java
test/hotspot/jtreg/compiler/loopopts/parallel_iv/TestParallelIvInIntCountedLoop.java
test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java
test/hotspot/jtreg/compiler/c2/aarch64/TestFloat16Replicate.java
test/hotspot/jtreg/compiler/vectorapi/TestVectorAddMulReduction.java
test/hotspot/jtreg/compiler/vectorapi/TestVectorCompressExpandBits.java
test/hotspot/jtreg/compiler/vectorapi/VectorMultiplyOpt.java
test/hotspot/jtreg/compiler/vectorapi/VectorSaturatedOperationsTest.java
test/hotspot/jtreg/compiler/vectorization/TestMacroLogicVector.java
eme64
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sarannat Thanks for working on this. Looks good 😊
Though you should also look at the suggestions from the others above ;)
Issue: Some compiler tests uses randomization but does not have
@key randomnessin the jtreg header.Fix: The list of test cases that did not have
@key randomnesswere listed usinggrep -l "getRandomInstance" -r test/hotspot/jtreg/compiler/ | xargs grep -L "randomness". This PR adds@key randomnessto these tests.Note: The following tests that are still listed with
grep -l "getRandomInstance" -r test/hotspot/jtreg/compiler/ | xargs grep -L "randomness"after this PR are confirmed to be helper or support file for actual test.test/hotspot/jtreg/compiler/codegen/aes/TestAESBase.java
test/hotspot/jtreg/compiler/compilercontrol/jcmd/StressAddJcmdBase.java
test/hotspot/jtreg/compiler/compilercontrol/parser/HugeDirectiveUtil.java
test/hotspot/jtreg/compiler/compilercontrol/share/scenario/CommandGenerator.java
test/hotspot/jtreg/compiler/lib/ir_framework/test/TestVM.java
test/hotspot/jtreg/compiler/lib/ir_framework/test/ArgumentValue.java
test/hotspot/jtreg/compiler/lib/ir_framework/AbstractInfo.java
test/hotspot/jtreg/compiler/lib/ir_framework/CompLevel.java
test/hotspot/jtreg/compiler/lib/generators/Generators.java
test/hotspot/jtreg/compiler/lib/template_framework/library/PrimitiveType.java
test/hotspot/jtreg/compiler/lib/template_framework/library/Expression.java
test/hotspot/jtreg/compiler/lib/template_framework/NameSet.java
test/hotspot/jtreg/compiler/intrinsics/mathexact/Verify.java
test/hotspot/jtreg/compiler/intrinsics/bmi/BMITestRunner.java
test/hotspot/jtreg/compiler/intrinsics/unsafe/ByteBufferTest.java
test/hotspot/jtreg/compiler/arraycopy/stress/StressBooleanArrayCopy.java
test/hotspot/jtreg/compiler/arraycopy/stress/StressIntArrayCopy.java
test/hotspot/jtreg/compiler/arraycopy/stress/StressLongArrayCopy.java
test/hotspot/jtreg/compiler/arraycopy/stress/StressCharArrayCopy.java
test/hotspot/jtreg/compiler/arraycopy/stress/StressObjectArrayCopy.java
test/hotspot/jtreg/compiler/arraycopy/stress/StressByteArrayCopy.java
test/hotspot/jtreg/compiler/arraycopy/stress/StressFloatArrayCopy.java
test/hotspot/jtreg/compiler/arraycopy/stress/StressShortArrayCopy.java
test/hotspot/jtreg/compiler/arraycopy/stress/StressDoubleArrayCopy.java
test/hotspot/jtreg/compiler/codecache/cli/codeheapsize/JVMStartupRunner.java
test/hotspot/jtreg/compiler/vectorapi/reshape/utils/VectorReshapeHelper.java
test/hotspot/jtreg/compiler/jvmci/compilerToVM/DummyClass.java
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/28463/head:pull/28463$ git checkout pull/28463Update a local copy of the PR:
$ git checkout pull/28463$ git pull https://git.openjdk.org/jdk.git pull/28463/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 28463View PR using the GUI difftool:
$ git pr show -t 28463Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/28463.diff
Using Webrev
Link to Webrev Comment