On 2016-03-16 19:17, Christian Thalinger wrote:
On Mar 15, 2016, at 10:51 PM, Erik Joelsson <erik.joels...@oracle.com> wrote:
The __FILE__ macro is built into the compiler and evaluates to the file name of
the current source file. It's intended use is for debug messages. In the rest
of the JDK we eliminated its use completely by providing a macro called
THIS_FILE that evaluates to the basename.ext of the source file being compiled.
In Hotspot it's still used and it's used in header files, which makes the
THIS_FILE solution invalid.
For most of our compilers, the format of the path is based on how the file was
presented on the command line. This matters to us since the old Hotspot build
uses relative paths to the generated source and header files and the new build
uses absolute paths for all source files. Also, the directory structure in the
output directory differ. This means that for certain object files, the constant
strings resulting from the use of __FILE__ differ, both in contents and length.
The difference in length causes alignment differences in the objects and the
resulting libraries.
This is actually a very good point. Alignment differences can cause
performance differences. Sometimes hard to track down and almost impossible to
reproduce.
When creating the new build, we are trying very hard to make sure we are still
building the same thing. One way to verify this is to compare various aspects
of the built binaries. When possible, this is far easier than running a very
large amount of tests and it gives us a large amount of confidence in our
changes. The alignment differences caused by different lengths of constant
strings severely limits the amount of clean comparisons we can do.
The particular fix here changes how we generate the files from the ADLC tool.
The tool already generates #line directives which helps the compiler figure out
which actual source file and line parts of the generated files came from.
Basically overriding what __FILE__ and __LINE__ will evaluate to. Some of these
lines need a bit of post processing, and for that, the build already uses awk
to rewrite them. The current awk script looks for lines like this, which the
ADLC tool prints when it doesn't know the correct source file:
#line 999999
and rewrites it to
#line <actual line number in generated file+1> <absolute path to generated file>
I changed this to instead rewrite to:
#line <actual line number in generated file> <basename.ext of generated file>
You removed the +1. Why was it used in the first place?
I honestly have no idea. The existing code adds 1, but from what I can
see, it's incorrect to do so. In my change, since I'm adding a new line
first in the file, it is correct to add 1 so I left that in the awk
script. That's what I mean with "actual line number in generated file"
without +1.
I also added an initial #line first in each file.
#line 1 <basename.ext of generated file>
With these changes, we can do very clean comparisons on Linux, Solaris and
Macosx. This helps the new hotspot build immensely, but will also be good in
the future as we have a very convenient way of verifying build changes that
shouldn't affect the built output.
Now this all makes sense. Thanks. The change looks good to me.
Thanks!
/Erik
/Erik
On 2016-03-15 22:02, Christian Thalinger wrote:
On Mar 15, 2016, at 12:06 AM, Erik Joelsson <erik.joels...@oracle.com> wrote:
Any chance I could get a second review on this? Preferably from the hotspot
team.
It’s not quite obvious to me what the __FILE__ change is. Maybe an example
would help.
/Erik
On 2016-03-11 18:16, Erik Joelsson wrote:
In preparation for the new Hotspot build, there are a few changes to the old
build that needs to happen first. These changes should be harmless, but do
impact the generated binaries some. These changes are:
* Standardizing sort order for objects on link command line on Windows to a
locale independent order.
* Working around compare differences caused by the __FILE__ macro in certain
generated files by overriding the value of __FILE__ in those files.
By making these changes first and separate from introducing the new build we
reduce the potential impact of when we do introduce the new build.
Bug: https://bugs.openjdk.java.net/browse/JDK-8151656
Webrev: http://cr.openjdk.java.net/~erikj/8151656/index.html
/Erik
