> 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 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.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 >
