Looks good to me as well.
Tim
On 03/02/18 07:29, Erik Joelsson wrote:
This looks good to me.
/Erik
On 2018-03-02 04:45, Magnus Ihse Bursie wrote:
On 2018-03-02 12:10, Magnus Ihse Bursie wrote:
On 2018-03-02 03:01, David Holmes wrote:
Hi Magnus,
On 1/03/2018 10:48 AM, Magnus Ihse Bursie wrote:
We're doing a lot of weird compilation stuff for dtrace. With this
patch, most of the weirdness is removed. The remaining calls to
$(CC) -E has been changed to $(CPP) to clarify that we do not
compile, we just use the precompiler.
One of the changes I made was to actually split up the last and
final dtrace call into a separate preprocessing step. However, this
uses the solaris studio preprocessor instead of the ancient system
preprocessor, which has changed behavior. A string like (&``_var)
is now expanded to (& ` ` _var), which is not accepted by dtrace.
:-( I have worked around this by adding the preprocessed output,
without the spaces, in two places. If anyone wants to dig deeper
into dtrace script file syntax, or C preprocessor magic, to avoid
this, let me know... (I'll just state that the "obvious" solution
of sending -Xs to the preprocessor to get old-style behavior does
not work: this just makes the solaris studio preprocessor call the
ancient preprocessor in turn, and we've gained nothing...)
Bug: https://bugs.openjdk.java.net/browse/JDK-8198862
WebRev:
http://cr.openjdk.java.net/~ihse/JDK-8198862-stop-doing-funky-dtrace-compilation-stuff/webrev.01
Why did you rename generateJvmOffsetsMain.c to
generateJvmOffsetsMain.cpp? It isn't a C++ program, it's just a C
program.
Yes, but so are generateJvmOffsets.cpp. :-& There was no point in
mixing a .cpp and .c file for this trivial build tool helper. In
fact, I don't even understand why they are two separate files -- if I
get the blessings from someone in hotspot, I'll gladly just
concatenate them into a single file.
Come to think about it, I don't care about the hotspot group's
blessing. ;-) I just moved the main function into the
generateJvmOffsets.cpp file. It was just silly having it as a separate
file.
! # Since we cannot generated JvmOffsets.cpp as part of the
gensrc step,
Comment doesn't read right.
Typo, should be "generate". I'll fix.
Updated.
I also restored the extra ( ) in ExecuteWithLog with redirection, and
added an additional ( ) for one case that was previously missing one.
Finally I also added the changes to dtrace that Erik requested for
JDK-8198859, but which was already pushed by that time.
New webrev:
http://cr.openjdk.java.net/~ihse/JDK-8198862-stop-doing-funky-dtrace-compilation-stuff/webrev.02
/Magnus
