Re: 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.joels...@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
     >