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




Reply via email to