RFR: 8247396: PCH should be disabled for opt-size feature with clang after JDK-8246751

[Magnus Ihse Bursie]

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