Hi Chris,
Thank you for replying!
I was going to wait for Adrian but it's a big enough change that it would bit rot if I waited too long, and I didn't know how long it would be.  We can fix anything that is disagreeable with a future patch, I think.  So I pushed it today.
Thanks,
Coleen

On 6/24/20 1:24 PM, Chris Phillips wrote:
Hi Coleen,

On 2020-06-23 16:35, coleen.phillim...@oracle.com wrote:
Including build-dev.

On 6/23/20 12:17 AM, David Holmes wrote:
Hi Coleen,

Cleanup is looking good but a few comments:

- if the bytecodeInterpreter is also zero-only can we rename its files
too? (I really find it hard to figure out which files are really
needed/used for a given build.)
You're right that bytecodeInterpreter* files in the interpreter
directory always confuses everybody.  So I moved them to
src/hotspot/share/interpreter/zero.  I also fixed the build to not build
that directory unless you're building zero.

I removed a few more unnecessary #includes.
- you are excluding shared templateInterpreter*.* from the zero build so
"#ifndef ZERO" is always true in those files.
The templateInterpreter header files still need #ifndef ZERO because
they can be transitively included in interpreter.hpp.  I removed #ifndef
ZERO from the .cpp files only.

- are the platform specific templateInterpreter* files already
excluded from a zero build? Otherwise they should be added to the
exclude list.
They are already excluded because zero doesn't build those platform files.

- how were you able to completely delete:
   - src/hotspot/share/interpreter/cppInterpreter.cpp
   - src/hotspot/share/interpreter/cppInterpreterGenerator.cpp
?
I inlined them into the cpu/zero/*_zero versions.

So the additional changes to move the bytecodeInterpreter* and zero
files to a zero directory are here.  I didn't want to move them to the
cpu/zero directory because they are not analogs to the other cpu files.

incremental:http://cr.openjdk.java.net/~coleenp/2020/8239782.02.incr/webrev/index.html

full: http://cr.openjdk.java.net/~coleenp/2020/8239782.02/webrev/

Tested with tier1 on Oracle platforms and zero product and fastdebug
build.  CC'ing Adrian who works on Zero, can you comment on this patch?

Thanks,
Coleen
Thanks,
David
----


On 23/06/2020 1:36 am, coleen.phillim...@oracle.com wrote:
Summary: Change CC_INTERP conditional to ZERO and remove in places
where unnecessary. Fix build to exclude compilers and rename
CppInterpreter to ZeroInterpreter. The "C++ Interpreter" has been
removed from the code a while ago.

The motivation is to remove CC_INTERP conditionals from common code
for the most part.  The C++ interpreter used to work with C1 and C2.
Some of the hooks are still present (can be cleaned out or
implemented correctly later) but I removed some other unconditionally
false code in order to remove interactions with common code.  Also it
appeared that Zero was creating method counters when it was never
using them.  I removed this too, hoping it would make zero faster,
but nope, it's still slow.

I also renamed cppInterpreter and CppInterpreter to zeroInterpreter
and ZeroInterpreter, respectively, and moved some code to cpu/zero.
Thus ends pass 10? of cleaning up this code.

Tested with tier1 on Oracle platforms and built these:
linux-arm32,linux-ppc64le-debug,linux-s390x-debug,linux-x64-zero,linux-x64-zero-debug.
If you work on Zero, can you give this a test run with your favorite
platform and review?

open webrev at
http://cr.openjdk.java.net/~coleenp/2020/8239782.01/webrev
bug link https://bugs.openjdk.java.net/browse/JDK-8239782

Thanks,
Coleen



I have gone through the changes also, and they look great. But agree
that it would be good if Adrian Glaubitz or someone at Debian could test
these in their test pool.

Unfortunately I don't know who to suggest.

Cheers!
Chris Phi

Reply via email to