8247396: PCH should be disabled for opt-size feature with clang after JDK-8246751(Internet mail)

[jiefu(傅杰)]

Thanks Magnus for your review and help.
I can push it myself and will do it later.

Best regards,
Jie

On 2020/6/12, 5:08 PM, "Magnus Ihse Bursie" <magnus.ihse.bursie at oracle.com> wrote:

    On 2020-06-12 04:58, jiefu(傅杰) wrote:
    > Hi Erik and Magnus,
    >
    > Thanks for your help.
    >
    > Updated: http://cr.openjdk.java.net/~jiefu/8247396/webrev.02/
    > Also I've changed the JBS title to 'Exclude PCH for files in $(OPT_SPEED_SRC) with clang after JDK-8246751'.
    >
    > Please review it and give me some adviece.
    Looks good to me!
    
    Do you need help sponsoring this?
    
    /Magnus
    >
    > Thanks a lot.
    > Best regards,
    > Jie
    >
    > On 2020/6/11, 9:54 PM, "Erik Joelsson" <erik.joelsson at oracle.com> wrote:
    >
    >      On 2020-06-11 06:41, Magnus Ihse Bursie wrote:
    >      > On 2020-06-11 14:34, Erik Joelsson wrote:
    >      >> I agree with the assessment, but I think the disabling should happen
    >      >> in configure. That way the configure log can accurately tell the user
    >      >> why it was disabled. This can be done by just adding another elif in
    >      >> BPERF_SETUP_PRECOMPILED_HEADERS. To check for a jvm feature in
    >      >> configure, you can use JVM_FEATURES_IS_ACTIVE(opt-size).
    >      > Actually, I'm not sure I agree with the assessment. Why does this only
    >      > happen for clang? Is it a problem when building with opt-size and pch
    >      > for other compilers?
    >      >
    >      > If not, that's likely due to the problematic files being excluded from
    >      > PCH (if I remember correctly). I believe a better way forward would be
    >      > to make sure that files that are compiled with a different
    >      > optimization level should be excluded from PCH for clang as well.
    >      >
    >      Right, I just took a look and in JvmOverrideFiles.gmk we build a list of
    >      files to exclude from precompiled header. We could just add
    >      $(OPT_SPEED_SRC) to this list (as that variable is defined earlier in
    >      JvmFeatures.gmk). OPT_SPEED_SRC is empty unless opt-size is true, so
    >      just adding it to the list should be fine.
    >      
    >      /Erik
    >      
    >      > /Magnus
    >      >>
    >      >> /Erik
    >      >>
    >      >> On 2020-06-11 02:25, jiefu(傅杰) wrote:
    >      >>> Hi all,
    >      >>>
    >      >>> JBS:    https://bugs.openjdk.java.net/browse/JDK-8247396
    >      >>> Webrev: http://cr.openjdk.java.net/~jiefu/8247396/webrev.00/
    >      >>>
    >      >>> -O3 is used for clang on MacOS after JDK-8246751.
    >      >>> But it breaks the builds of VMs with opt-size feature when PCH is
    >      >>> enabled.
    >      >>>
    >      >>> The reason is that the PCH was built with -Os while some other files
    >      >>> were built with -O3, which causes an error with clang.
    >      >>> It would be better to disable the PCH for opt-size builds.
    >      >>>
    >      >>> Thanks a lot.
    >      >>> Best regards,
    >      >>> Jie
    >      >
    >      
    >      
    >