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

[Erik Joelsson]

Hello,

For this to work in the current set of elifs you need to either combine 
the checks for clang and opt-size into one conditional, so the final 
else is still reached if toolchain is clang but opt-size is not present, 
or add another else to the nested if statement like in the block above 
for gcc. I prefer the former in this case.

/Erik

On 2020-06-11 06:10, jiefu(傅杰) wrote:
Hi Erik,

Thanks for your review and valuable comments.

Updated: http://cr.openjdk.java.net/~jiefu/8247396/webrev.01/

Please review it and give me some adviece.

Thanks a lot.
Best regards,
Jie

On 2020/6/11, 8:37 PM, "Erik Joelsson" <erik.joels...@oracle.com> 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).
     
     /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