Code Review for 6965570

Prepared by:never on Tue Mar 1 10:30:33 PST 2011
Compare against: ssh://
Summary of changes: 50 lines changed: 8 ins; 20 del; 22 mod; 10146 unchg
Patch of changes: 6965570.patch
Author comments:
6965570: assert(!needs_patching && x->is_loaded(),"how do we know it's volatile if it's not loaded")

During code generation for a volatile getfield in an outer class we've
managed to resolve the inner class symbolically but the constant pool
reference hasn't actually been resolved. This results in a field
reference that should be patched so that the proper access checks are
performed by constant pool resolution. In this case the field is
volatile and LIRGenerator expects that if the class was loaded and we
can see that's it actually volatile that a patch must not be needed.
In debug mode this asserts but in product mode it generates code to
uses max_jint for the field offset. The fix is to always respect the
needs_patching flag. We could makes the patching machinery handle
this more normally but that would complicate patching even more. It's
a rare case so allowing deopt in Runtime1::patch_code to handle it is
the easiest way to go.

Part of the confusion stems from the is_loaded flag on AccessField
which doesn't really mean what it says, so I've removed it and pushed
all the needs_patching logic up into GraphBuilder. is_initialized()
is similarly confusing since it really means does this static field
need patching. I've eliminated that flag from AccessField and
captured that logic in a new method that is used instead. Tested with
runthese with and without PatchALot, ctw, and the nsk jvmti tests.

Legend: Modified file
Deleted file
New file

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/c1/c1_Instruction.hpp

24 lines changed: 6 ins; 12 del; 6 mod; 2336 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/c1/c1_GraphBuilder.cpp

15 lines changed: 0 ins; 4 del; 11 mod; 3802 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/c1/c1_LIRGenerator.cpp

6 lines changed: 0 ins; 4 del; 2 mod; 2872 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/c1/c1_ValueMap.hpp

4 lines changed: 2 ins; 0 del; 2 mod; 236 unchg

Cdiffs Udiffs Sdiffs Frames Old New Patch Raw src/share/vm/c1/c1_Canonicalizer.cpp

1 line changed: 0 ins; 0 del; 1 mod; 900 unchg

This code review page was prepared using /never/bin/webrev (vers 23.18-hg-never).