On 16/10/2017 3:49 PM, Roman Kennke wrote:
Hi David,
thanks for reviewing and testing!
The interaction between JEPs and patches going in is not really clear to
me, nor is it well documented. For example, we're already pushing
patches for JEP 304: Garbage Collection Interface, even though it's only
in 'candidate' state...
If patches can be separated out into generally useful cleanup or
enabling changes then it can be okay to push them independently of the
JEP AFAIK. That's obviously a little subjective. In this case though
we're talking about the whole thing at once, so AFAIK the JEP has to be
targeted before the changes can be pushed.
In any case, I'll ping Mark Reinhold about moving the Shark JEP forward.
Thanks. Should be simple enough, I hope. :)
Cheers,
David
Thanks again,
Roman
My internal JPRT run went fine. So this just needs a build team
signoff from the perspective of the patch.
However, as this has had a JEP submitted for it, the code changes can
not be pushed until the JEP has been targeted.
Thanks,
David
On 16/10/2017 8:08 AM, David Holmes wrote:
Looks good.
Thanks,
David
On 16/10/2017 8:00 AM, Roman Kennke wrote:
Ok, I fixed all the comments you mentioned.
Differential (against webrev.01):
http://cr.openjdk.java.net/~rkennke/8171853/webrev.03.diff/
<http://cr.openjdk.java.net/%7Erkennke/8171853/webrev.03.diff/>
Full webrev:
http://cr.openjdk.java.net/~rkennke/8171853/webrev.03/
<http://cr.openjdk.java.net/%7Erkennke/8171853/webrev.03/>
Roman
Just spotted this:
./hotspot/jtreg/compiler/whitebox/CompilerWhiteBoxTest.java: /**
{@code CompLevel::CompLevel_full_optimization} -- C2 or Shark */
David
On 16/10/2017 7:25 AM, David Holmes wrote:
On 16/10/2017 7:01 AM, Roman Kennke wrote:
Hi David,
thanks!
I'm uploading a 2nd revision of the patch that excludes the
generated-configure.sh part, and adds a smallish Zero-related fix.
http://cr.openjdk.java.net/~rkennke/8171853/webrev.01/
<http://cr.openjdk.java.net/%7Erkennke/8171853/webrev.01/>
Can you point me to the exact change please as I don't want to
re-examine it all. :)
I'll pull this in and do a test build run internally.
Thanks,
David
Thanks, Roman
Hi Roman,
The build changes must be reviewed on build-dev - now cc'd.
Thanks,
David
On 15/10/2017 8:41 AM, Roman Kennke wrote:
The JEP to remove the Shark compiler has received exclusively
positive feedback (JDK-8189173) on zero-dev. So here comes the
big patch to remove it.
What I have done:
grep -i -R shark src
grep -i -R shark make
grep -i -R shark doc
grep -i -R shark doc
and purged any reference to shark. Almost everything was
straightforward.
The only things I wasn't really sure of:
- in globals.hpp, I re-arranged the KIND_* bits to account for
the gap that removing KIND_SHARK left. I hope that's good?
- in relocInfo_zero.hpp I put a ShouldNotCallThis() in
pd_address_in_code(), I am not sure it is the right thing to
do. If not, what *would* be the right thing?
Then of course I did:
rm -rf src/hotspot/share/shark
I also went through the build machinery and removed stuff
related to Shark and LLVM libs.
Now the only references in the whole JDK tree to shark is a
'Shark Bay' in a timezone file, and 'Wireshark' in some tests ;-)
I tested by building a regular x86 JVM and running JTREG tests.
All looks fine.
- I could not build zero because it seems broken because of the
recent Atomic::* changes
- I could not test any of the other arches that seemed to
reference Shark (arm and sparc)
Here's the full webrev:
http://cr.openjdk.java.net/~rkennke/8171853/webrev.00/
<http://cr.openjdk.java.net/%7Erkennke/8171853/webrev.00/>
Can I get a review on this?
Thanks, Roman