RFR (L, tedious) 8199809: Don't include frame.inline.hpp and other.inline.hpp from .hpp files

David Holmes david.holmes at oracle.com
Wed Mar 21 22:34:06 UTC 2018


<trimming>

On 22/03/2018 1:35 AM, Thomas Stüfe wrote:
> On Wed, Mar 21, 2018 at 2:02 PM, David Holmes <david.holmes at oracle.com 
>     On 21/03/2018 10:39 PM, coleen.phillimore at oracle.com
>         Hm so I need to add the #include for macroAssembler.hpp
>         somewhere new like nativeInst_ppc.hpp or does just removing it
>         from interpreterRT_ppc.hpp fix the problem?
> 
> 
>     Whatever code is in the included platform specific header still
>     needs to ensure the definitions that it needs have been included. If
>     those are shared files then you may just be able to move them into
>     the shared cpp file, but any platform specific headers must still be
>     included in the platform specific headers.
> 
> 
> I disagree in this particular case. In my opinion, headers whose purpose 
> is to be included into class declarations should not include other headers.

??? If the code you are including relies on things from other header 
files then you have no choice else it won't compile!

David

> Thanks, Thomas
> 
>     David
>     -----
> 
>         thanks,
>         Coleen
> 
> 
> 
>             Thanks, Thomas
> 
> 
> 
> 
> 
>             On Wed, Mar 21, 2018 at 11:41 AM, Thomas Stüfe
>             <thomas.stuefe at gmail.com <mailto:thomas.stuefe at gmail.com>
>             <mailto:thomas.stuefe at gmail.com
>             <mailto:thomas.stuefe at gmail.com>>> wrote:
> 
>                  Hi Coleen,
> 
>                  linuxs390 needs this:
> 
>                  - .../source $ hg diff
>                  diff -r daf3abb9031f
>             src/hotspot/cpu/s390/interpreterRT_s390.cpp
>                  --- a/src/hotspot/cpu/s390/interpreterRT_s390.cpp    
>               Wed Mar 21
>                  08:37:04 2018 +0100
>                  +++ b/src/hotspot/cpu/s390/interpreterRT_s390.cpp    
>               Wed Mar 21
>                  11:12:03 2018 +0100
>                  @@ -65,7 +65,7 @@
>                   }
> 
>                   // Implementation of SignatureHandlerGenerator
>                 
>             -InteprerterRuntime::SignatureHandlerGenerator::SignatureHandlerGenerator(
> 
>                 
>             +InterpreterRuntime::SignatureHandlerGenerator::SignatureHandlerGenerator(
> 
>                       const methodHandle& method, CodeBuffer* buffer) :
>                  NativeSignatureIterator(method) {
>                     _masm = new MacroAssembler(buffer);
>                     _fp_arg_nr = 0;
> 
>                  (typo). Otherwise it builds fine.
> 
>                  I'm getting build errors on AIX which are a bit more
>             complicated,
>                  still looking..
> 
>                  Thanks, Thomas
> 
> 
>                  On Wed, Mar 21, 2018 at 1:08 AM,
>             <coleen.phillimore at oracle.com
>             <mailto:coleen.phillimore at oracle.com>
>                  <mailto:coleen.phillimore at oracle.com
>             <mailto:coleen.phillimore at oracle.com>>> wrote:
> 
>                      Summary: Remove frame.inline.hpp,etc from header
>             files and
>                      adjust transitive includes.
> 
>                      Tested with mach5 tier1 on Oracle platforms: linux-x64,
>                      solaris-sparc, windows-x64.  Built with open-only
>             sources
>                      using --disable-precompiled-headers on linux-x64,
>             built with
>                      zero (also disable precompiled headers). Roman
>             built with
>                      aarch64, and have request to build ppc, etc. 
>             (Please test
>                      this patch!)
> 
>                      Semi-interesting details:  moved
>             SignatureHandlerGenerator
>                      constructor to cpp file, moved
>                      interpreter_frame_stack_direction() to target
>             specific hpp
>                      files (even though they're all -1), pd_last_frame to
>                      thread_<os_cpu>.cpp because there isn't a
>                      thread_<os_cpu>.inline.hpp file, lastly moved
>                      InterpreterRuntime::LastFrameAccessor into
>                      interpreterRuntime.cpp file, and a few other
>             functions moved
>                      in shared code.
> 
>                      This is the last of this include file technical
>             debt cleanup
>                      that I'm going to do.  See bug for more information.
> 
>                      open webrev at
>             http://cr.openjdk.java.net/~coleenp/8199809.01/webrev
>             <http://cr.openjdk.java.net/~coleenp/8199809.01/webrev>
>                     
>             <http://cr.openjdk.java.net/%7Ecoleenp/8199809.01/webrev
>             <http://cr.openjdk.java.net/%7Ecoleenp/8199809.01/webrev>>
>                      bug link
>             https://bugs.openjdk.java.net/browse/JDK-8199809
>             <https://bugs.openjdk.java.net/browse/JDK-8199809>
>                      <https://bugs.openjdk.java.net/browse/JDK-8199809
>             <https://bugs.openjdk.java.net/browse/JDK-8199809>>
> 
>                      I'll update the copyrights when I commit.
> 
>                      Thanks,
>                      Coleen
> 
> 
> 
> 
> 


More information about the hotspot-dev mailing list