RFR: JDK-8151656: Minor tweaks to old Hotspot build to ease comparison with new

[Christian Thalinger]

> On Mar 15, 2016, at 10:51 PM, Erik Joelsson <erik.joelsson at 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 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.

> 
> /Erik
> 
> On 2016-03-15 22:02, Christian Thalinger wrote:
>>> On Mar 15, 2016, at 12:06 AM, Erik Joelsson <erik.joelsson at 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
>