RE: [PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

2020-02-14 Thread Jinsong Ji via cfe-commits 
Thanks Melanie.

I have pushed one fix to test-suite to explicitly add -ffp-contract=off to
unblock our internal buildbot for now.
https://github.com/llvm/llvm-test-suite/commit/c04a7178a3a50fe919964df59b041c5671db50f7

Our buildbot are OK now.

I think you can proceed as long as the change is intended and reasonable.
Thanks.


Best,

Jinsong Ji (纪金松), PhD.

XL/LLVM on Power Compiler Development
E-mail: j...@us.ibm.com



From:   "Blower, Melanie I" 
To: "reviews+d74436+public+e2b40a7853ffb...@reviews.llvm.org"
,
"lebedev...@gmail.com" ,
"rjmcc...@gmail.com" ,
"sepavl...@gmail.com" 
Cc: "mask...@google.com" , "j...@us.ibm.com"
, "david.bolvan...@gmail.com"
, "mar...@martin.st"
, "Wang, Pengfei" ,
"wuz...@cn.ibm.com" ,
"nemanja.i@gmail.com" ,
"kit.bar...@gmail.com" ,
"cfe-commits@lists.llvm.org" ,
"mlek...@skidmore.edu" ,
"blitzrak...@gmail.com" ,
    "shen...@google.com" ,
        "peter.wal...@arm.com" 
Date:   02/14/2020 10:34 AM
Subject:[EXTERNAL] RE: [PATCH] D74436: Change clang option
-ffp-model=precise to select ffp-contract=on



I reverted MaskRay's "reland" since the original patch is causing trouble
on PowerPC, check-all is passing on my box.  Sorry for the trouble.

> -Original Message-
> From: Andy Kaylor via Phabricator 
> Sent: Thursday, February 13, 2020 9:20 PM
> To: Blower, Melanie I ; lebedev...@gmail.com;
> rjmcc...@gmail.com; sepavl...@gmail.com
> Cc: mask...@google.com; j...@us.ibm.com; david.bolvan...@gmail.com;
> mar...@martin.st; Wang, Pengfei ;
> wuz...@cn.ibm.com; nemanja.i@gmail.com; kit.bar...@gmail.com; cfe-
> comm...@lists.llvm.org; mlek...@skidmore.edu; blitzrak...@gmail.com;
> shen...@google.com; peter.wal...@arm.com
> Subject: [PATCH] D74436: Change clang option -ffp-model=precise to select
ffp-
> contract=on
>
> andrew.w.kaylor added a subscriber: MaskRay.
> andrew.w.kaylor added a comment.
>
> In D74436#1875386 <
https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D74436-231875386=DwIFAg=jf_iaSHvJObTbx-siA1ZOg=DvnnfavFQBGT2CDyHzTr_Q=cvHv8MkmryQUMVKDW_JEP3rPsAVn_T77lN-oqkY9X2Y=4moq7_F2LQUHDL0EsCHrNKUtm43d0NQb2Fmh4CISYOM=
 >, @thakis
> wrote:
>
> > The revert of this breaks tests everywhere, as far as I can tell.
>
>
> It looks like something strange happened with the revert:
>
> > clang-11: warning: overriding '-ffp-model=strict' option with '-ffp-
> model=strict' [-Woverriding-t-option]
>
> I believe the problem is that the original change that was being reverted
> contained this:
>
>   clang/lib/Driver/ToolChains/Clang.cpp
>   @@ -2768,7 +2766,7 @@ static void RenderFloatingPointOptions(const
> ToolChain , const Driver ,
>   !AssociativeMath && !ReciprocalMath &&
>   SignedZeros && TrappingMath && RoundingFPMath &&
>   DenormalFPMath != llvm::DenormalMode::getIEEE() &&
>   +FPContract.empty())
>   -(FPContract.equals("off") || FPContract.empty()))
>
> But sometime in the land-revert-land-revert cycle the line above that
changed,
> causing the merge to miss this change in the most recent revert. I see
that
> @MaskRay has since re-landed this change set, but it's going to cause
problems
> for PowerPC. If someone needs to revert this yet again, I think it can be
safely
> done by recovering the change above.
>
> Apologies for the mess!
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D74436_new_=DwIFAg=jf_iaSHvJObTbx-siA1ZOg=DvnnfavFQBGT2CDyHzTr_Q=cvHv8MkmryQUMVKDW_JEP3rPsAVn_T77lN-oqkY9X2Y=x-uw-PdxKFtF2QXI5p8pFIGwDP53ma6WcFfJSt7NiPY=

>
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D74436=DwIFAg=jf_iaSHvJObTbx-siA1ZOg=DvnnfavFQBGT2CDyHzTr_Q=cvHv8MkmryQUMVKDW_JEP3rPsAVn_T77lN-oqkY9X2Y=VJzMi9ZvFzcqz_BCC84nlLx_A4agCXEmi9bNsJMnkJQ=

>
>



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

RE: [PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

2020-02-14 Thread Blower, Melanie I via cfe-commits 
I reverted MaskRay's "reland" since the original patch is causing trouble on 
PowerPC, check-all is passing on my box.  Sorry for the trouble. 

> -Original Message-
> From: Andy Kaylor via Phabricator 
> Sent: Thursday, February 13, 2020 9:20 PM
> To: Blower, Melanie I ; lebedev...@gmail.com;
> rjmcc...@gmail.com; sepavl...@gmail.com
> Cc: mask...@google.com; j...@us.ibm.com; david.bolvan...@gmail.com;
> mar...@martin.st; Wang, Pengfei ;
> wuz...@cn.ibm.com; nemanja.i@gmail.com; kit.bar...@gmail.com; cfe-
> comm...@lists.llvm.org; mlek...@skidmore.edu; blitzrak...@gmail.com;
> shen...@google.com; peter.wal...@arm.com
> Subject: [PATCH] D74436: Change clang option -ffp-model=precise to select ffp-
> contract=on
> 
> andrew.w.kaylor added a subscriber: MaskRay.
> andrew.w.kaylor added a comment.
> 
> In D74436#1875386 , @thakis
> wrote:
> 
> > The revert of this breaks tests everywhere, as far as I can tell.
> 
> 
> It looks like something strange happened with the revert:
> 
> > clang-11: warning: overriding '-ffp-model=strict' option with '-ffp-
> model=strict' [-Woverriding-t-option]
> 
> I believe the problem is that the original change that was being reverted
> contained this:
> 
>   clang/lib/Driver/ToolChains/Clang.cpp
>   @@ -2768,7 +2766,7 @@ static void RenderFloatingPointOptions(const
> ToolChain , const Driver ,
>   !AssociativeMath && !ReciprocalMath &&
>   SignedZeros && TrappingMath && RoundingFPMath &&
>   DenormalFPMath != llvm::DenormalMode::getIEEE() &&
>   +FPContract.empty())
>   -(FPContract.equals("off") || FPContract.empty()))
> 
> But sometime in the land-revert-land-revert cycle the line above that changed,
> causing the merge to miss this change in the most recent revert. I see that
> @MaskRay has since re-landed this change set, but it's going to cause problems
> for PowerPC. If someone needs to revert this yet again, I think it can be 
> safely
> done by recovering the change above.
> 
> Apologies for the mess!
> 
> 
> Repository:
>   rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D74436/new/
> 
> https://reviews.llvm.org/D74436
> 
> 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits