RFR: 8003310: Enable -Wunused when compiling with GCC (original) (raw)

Mikael Vidstedt mikael.vidstedt at oracle.com
Thu Nov 15 16:20:47 PST 2012

On 2012-11-12 21:47, David Holmes wrote:

Hi Mikael,

A couple of general observations as really the "owners" of each file needs to assess the changes: - sometimes functions exist for debugging/tracing and calls will be added to the code by engineers as they debug. For example MBFence in synchronizer.cpp allows you to add fences into expressions.

For MBFence specifically, given that it is a static in synchronizer.cpp I'm going to assume it's a leftover and any future debugging/tracing should be able to do a call to OrderAccess::fence() directly instead, but in general you're absolutely right and I'm counting on the owners to point out if there are functions that are frequently used during development.

Put differently - is anybody counting on the following functions, if not they will be removed:

src/cpu/x86/vm/x86_64.ad:Address build_address(int b, int i, int s, int d) src/cpu/x86/vm/methodHandles_x86.cpp:RegisterOrConstant constant(int value) src/cpu/x86/vm/assembler_x86.cpp:int encode(XMMRegister r) src/share/vm/c1/c1_LIRGenerator.cpp:Value maxvalue(IfOp* ifop) src/share/vm/compiler/compileLog.cpp:const char* split_attrs(const char* &kind, char* buffer) src/share/vm/compiler/compilerOracle.cpp:const char * command_name(OracleCommand command) src/share/vm/gc_implementation/g1/ptrQueue.cpp:int byte_index_to_index(int ind) src/share/vm/gc_implementation/g1/ptrQueue.cpp:int index_to_byte_index(int byte_ind) src/share/vm/interpreter/interpreterRuntime.cpp:void trace_locking(Handle& h_locking_obj, bool is_locking) src/share/vm/memory/heap.cpp:size_t align_to_allocation_size(size_t size) src/share/vm/opto/connode.cpp:bool can_cause_alias(Node *n, PhaseTransform *phase) src/share/vm/opto/subnode.cpp:Node *clone_cmp( Node *cmp, Node *cmp1, Node *cmp2, PhaseGVN *gvn, BoolTest::mask test ) src/share/vm/prims/jni.cpp:methodHandle jni_resolve_interface_call(Handle recv, methodHandle method, TRAPS) src/share/vm/prims/jni.cpp:methodHandle jni_resolve_virtual_call(Handle recv, methodHandle method, TRAPS)

- why was the "static" removed from a number functions. They now have global visibility rather than being restricted to their files?

GCC knows that static functions declared in a file but not used in that same file are dead, and it emits a warning for each of them. My assumption was that the three functions for which I removed the static keyword are being used for debugging purposes, called from, say, gdb. It would be very helpful to get feedback on if that assumption is correct.

Or again, put differently - is anybody using any the following functions from a debugger. If I don't get confirmation on that they'll also be removed.

src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp:char* region_num_to_mbs(int length) src/share/vm/opto/block.cpp:void edge_dump(GrowableArray<CFGEdge *> *edges) src/share/vm/opto/block.cpp:void trace_dump(Trace *traces[], int count)

In globaleDefinitions.cpp:

+ void GlobalDefinitions::testglobals() { + intptrt pagesize = 4096; Page size may not be 4K - will the test still be valid?

I'll add tests with a couple of other page sizes as well!

The comments describing clampaddressinpage don't need to be on both the declaration and definition.

I'll keep it on the declaration only.

Cheers, David ------ On 13/11/2012 1:59 PM, Mikael Vidstedt wrote:

All, Please review the below change. The change adds the -Wunused flag when compiling with GCC and removes a number of unused functions/dead code. In the process I found one function (samepage) which was duplicated in four different places. I merged it to a single function, renamed it to clampaddressinpage, added some comments and refactored it to be slightly easier to understand. I also added unit tests for it. Feedback appreciated (especially on the name). http://cr.openjdk.java.net/~mikael/8003310/webrev.00/ Passes JPRT and the built-in unit tests (-XX:+ExecuteInternalVMTests). Thanks, Mikael

More information about the hotspot-dev mailing list