RFR(S): 8199698: Change 8199275 breaks template instantiation for xlC (and potentially other compliers)

[Volker Simonis]

Hi,

can I please have a review for the following small fix:

http://cr.openjdk.java.net/~simonis/webrevs/2018/8199698/
https://bugs.openjdk.java.net/browse/JDK-8199698

The fix is actually trivial: just defining the corresponding
"NOINLINE" macro for xlC. Unfortunately it's syntax requirements are a
little different for method declarations, compared to the other
platforms (it has to be placed AFTER the method declarator instead of
BEFORE it). Fortunately, there are no differences for method
definitions, so we can safely move the NOINLINE attributes from the
method declarations in allocation.hpp to the method definitions in
allocation.inline.hpp.

Thank you and best regards,
Volker

PS: or true C++ enthusiasts I've also included the whole story of why
this happens and why it happens just now, right after 8199275 :)

Change "8199275: Fix inclusions of allocation.inline.hpp" replaced the
inclusion of "allocation.inline.hpp" in some .hpp files (e.g.
constantPool.hpp) by "allocation.hpp".

"allocation.inline.hpp" contains not only the definition of some
inline methods (as the name implies) but also the definition of some
template methods (notably the various CHeapObj<>::operator new()
versions).

Template functions are on orthogonal concept with regard to inline
functions, but they share on implementation communality: at their call
sites, the compiler absolutely needs the corresponding function
definition. Otherwise it can either not inline the corresponding
function in the case of inline functions or it won't even be able to
create the corresponding instantiation in the case of a template
function.

Because of this reason, template functions and methods are defined in
their corresponding .inline.hpp files in HotSpot (even if they are not
subject to inlining). This is especially true for the before mentioned
CHeapObj<>:: new operators, which are explicitly marked as "NOINLINE"
in allocation.hpp but defined in allocation.inline.hpp.

Now every call site of these CHeapOb<>::new() operators which only
includes "allocation.hpp" will emit a call to the corresponding
instantiation of the CHeapObj<>:: new operator, but wont be able to
actually create that instantiation (simply because it doesn't see the
corresponding definition in allocation.inline.hpp). On the other side,
call sites of a CHeapObj<>:: new operator which include
allocation.inline.hpp will instantiate the required version in the
current compilation unit (or even inline that method instance if it is
not flagged as "NOINLINE").

If a compiler doesn't honor the "NOINLINE" attribute (or has an empty
definition for the NOINLIN macro like xlC), he can potentially inline
all the various template instances of CHeapObj<>:: new at all call
sites, if their implementation is available. This is exactly what has
happened on AIX/xlC before change 8199275 with the effect that the
resulting object files contained no single instance of the
corresponding new operators.

After change 8199275, the template definition of the CHeapObj<>:: new
operators aren't available any more at all call sites (because the
inclusion of allocation.inline.hpp was removed from some other .hpp
files which where included transitively before). As a result, the xlC
compiler will emit calls to the corresponding instantiations instead
of inlining them. But at all other call sites of the corresponding
operators, the operator instantiations are still inlined (because xlC
does not support "NOINLINE"), so we end up with link errors in
libjvm.so because of missing CHeapObj<>::new instances.

 As a general rule of thumb, we should always make template method
definitions available at all call sites, by placing them into
corresponding .inline.hpp files and including them appropriately.
Otherwise, we might end up without the required instantiations at link
time.

Unfortunately, there's no compile time check to enforce this
requirement. But we can misuse the "inline" keyword here, by
attributing template functions/methods as "inline". This way, the
compiler will warn us, if a template definition isn't available at a
specific call site. Of course this trick doesn't work if we
specifically want to define template functions/methods which shouldn't
be inlined, like in the current case :)