RFR (M): 8144748: Move assembler/macroAssembler inline function definitions to corresponding inline.hpp files
Vladimir Kozlov
vladimir.kozlov at oracle.com
Fri Dec 4 21:02:41 UTC 2015
> Btw, does anybody know why ret & retl are only inlined in PRODUCT builds?
TraceJumps is develop flag which is const 'false' in product (and in
optimized too). As result the code of ret() is only one instruction.
I think they put code in .cpp because wanted to set breakpoints in these
methods in debug VM but debug VM does not inline anyway. I think it
should be only in .hpp. Please, remove ifdef and code's copy in .cpp.
Also use code stile with {} and separate lines for:
+ inline void MacroAssembler::mov( Register s, Register d) {
+ if ( s != d ) or3( G0, s, d);
+ else assert_not_delayed(); // Put something useful in
the delay slot!
+ }
+
+ inline void MacroAssembler::mov_or_nop( Register s, Register d) {
+ if ( s != d ) or3( G0, s, d);
+ else nop();
+ }
Thanks,
Vladimir
On 12/4/15 12:23 PM, Mikael Vidstedt wrote:
>
> Please review this change which moves a large-ish number of function
> definitions/bodies from assembler_sparc.hpp and macroAssembler_sparc.hpp
> to the corresponding assembler_sparc.inline.hpp and
> macroAssembler_sparc.inline.hpp files.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8144748
> Webrev:
> http://cr.openjdk.java.net/~mikael/webrevs/8144748/webrev.00/webrev/
>
>
> * Background
>
> The specific problem which triggered this change was the following
> pattern in assembler_sparc.hpp:
>
> class Assembler : ... {
> ...
> inline void emit_int32(int);
> inline void emit_data(int x) { emit_data(x); }
> ...
> }
>
> If assembler_sparc.hpp is ever included directly without including
> assembler_sparc.inline.hpp this will lead to a use without definition,
> since emit_int32 is only defined in assembler_sparc.inline.hpp. The same
> pattern is true for almost all of the inline functions in
> assembler_sparc/macroAssembler_sparc.
>
> In general, inline functions (apart from trivial ones) should be defined
> in the inline.hpp file and any .cpp file actually making use of them
> should include the inline.hpp file instead. In this specific case, for
> whatever reason, it seems to be working well with Solaris Studio, but
> GCC is generating an error.
>
>
> * About the change
>
> The change here is very mechanical:
>
> for every relevant function F:
> * add "inline" keyword if needed
> * copy function to inline.hpp - trying to place it in the Right Place(tm)
> * add class prefix (Assembler:: or MacroAssembler::)
> * remove potential default parameter values
> * remove function body from .hpp file
>
> In a few cases I took the liberty of updating the indentation where it
> seemed off.
>
> Btw, does anybody know why ret & retl are only inlined in PRODUCT builds?
>
> Cheers,
> Mikael
>
More information about the hotspot-compiler-dev
mailing list