RFR(M): 8202713: Create a MacroAssembler::access_load/store_at wrapper for S390 and PPC

[Doerr, Martin]

Hi Götz,

> barrierSetAssembler_ppc.cpp
> 
> The name of the label is_null is misleading. It sounds
> as the name of a boolean variable. Maybe "L_handle_null"?
> And please use explicit compare for pointers: if (is_null == NULL) {
>
Done.

> Shouldn't there be an assertion that no other decorators
> are set than those handled here? Like all the memory ordering
> ones?
> 
Done.

> macroAssembler_ppc.hpp:
> 
> This header uses DecoratorSet thus should #include accessDecorators.hpp
> 
Done.

> access_load/store_at() 
> This is a misleading name for this function. I would think this 
> should go to nativeInst_ppc.cpp :)  Why would you access a store??
> But this needs to be changed on all platforms. So not 
> scope of this review.
> The function is not really needed. Make it private, or inline
> it into load/store_heap_oop.
> 
Made private.

> load_heap_oop_not_null:
> Please remove this on ppc, too. It's the only platform
> that still uses this function. There are only 6 uses.
> Please also remove the dead definitions of it on aarch64 and x86, 
> as well as the comment in s390.ad mentioning it. (not sure about
> aarch64 as you can't easily test this).
> 
Changed PPC64 and s390 code to use the full featured version only. Removed old functions.
I think it would be kind of unfair to change x86/aarch64 in this change because x86 folks will probably not read this review.

> Please extend the comment above load_heap_oop. It gained
> new functionality, and after removing the _not_null functions
> along with their documentation the current comment is pointless.
> Mention what the label is good for. Rename the label as above.
> 
Done.

> macroAssembler_ppc.inline.hpp
> 
> Please rename the boolean variable as_raw to without_gc_barrier.
> Actually, the DecoratorSet name AS_RAW is misleading, there is still
> encode/decode etc.  But this is not in the scope of this change.
> 
Not done because I'd like to keep PPC64 and s390 implementation consistent to the other platforms. A cleanup change for all platforms could be done in a separate change without s390 or PPC in its title.

> stubGenerator_ppc.cpp
> Remove comment // Load the oop. Makes the line too long, and 
> not too much information gained :)
> 
Done.

New webrev:
http://cr.openjdk.java.net/~mdoerr/8202713_ppc64_s390_masm_access/webrev.01/

Best regards,
Martin