[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

mibintc added a comment. In D81869#2116752 , @yaxunl wrote: > Would you please add the following lit test > > F12245277: diff.pch.txt > > If you change FastMath, FiniteMathOnly and UnsafeFPMath to > COMPATIBLE_LANGOPT,

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

yaxunl added a comment. Would you please add the following lit test F12245277: diff.pch.txt If you change FastMath, FiniteMathOnly and UnsafeFPMath to COMPATIBLE_LANGOPT, the test should pass. Comment at: clang/include/clang/Basic/LangOpt

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

rjmccall added a comment. In D81869#2112440 , @mibintc wrote: > I decided that I shouldn't make float options that define a macro, like > -ffast-math, as BENIGN_LANGOPT, I made ffp-contract= , fp-exception-behavior > and rounding-mode BENIGN That seems

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

mibintc updated this revision to Diff 273133. mibintc added a comment. I decided that I shouldn't make float options that define a macro, like -ffast-math, as BENIGN_LANGOPT, I made ffp-contract= , fp-exception-behavior and rounding-mode BENIGN, I modified the pch test case to test that the beni

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

rjmccall added a comment. Just a bunch of minor suggestions. LGTM if you get all the tests worked out and it actually works the way you want on the tests. Comment at: clang/include/clang/AST/Expr.h:2280 } + FPOptionsOverride getFPFeatures(const LangOptions &LO) const { +

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

mibintc added a comment. I need to make another revision that makes a couple more of the fp options, like ffp-contract, "benign" and add a test case that demonstrates fp options don't carry over from pch-create to pch-use Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

mibintc updated this revision to Diff 272808. mibintc added a comment. The difference between this patch and the earlier one today is that I modified the new test case I misunderstood why the test case was failing. The BENIGN_LANGOPT is working as desired and those pch diagnostics are no longer

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

mibintc updated this revision to Diff 272798. mibintc added a comment. I responded to review from @riccibruno and @rjmccall : I put the dump() routines into the .cpp file and changed them to use the x-macros. I moved CXXOperatorCall.FPOptionsOverride from the Expr bits into the OperatorCall it

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

rjmccall added inline comments. Comment at: clang/include/clang/AST/Stmt.h:620 +//unsigned FPFeatures : 21; +unsigned FPFeatures : 32; }; riccibruno wrote: > mibintc wrote: > > This is a temporary change, I know you don't want me to change the assert

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

riccibruno added inline comments. Comment at: clang/include/clang/AST/Stmt.h:620 +//unsigned FPFeatures : 21; +unsigned FPFeatures : 32; }; mibintc wrote: > This is a temporary change, I know you don't want me to change the assert to > allow a wider s

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

mibintc marked 6 inline comments as done. mibintc added a subscriber: yaxunl. mibintc added a comment. @rjmccall I added some inline questions and comments for you. Thanks a lot for your review. Comment at: clang/include/clang/AST/ExprCXX.h:172 + FPOptionsOverride getFPFeatu

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

mibintc updated this revision to Diff 272536. mibintc added a comment. Herald added a subscriber: martong. Herald added a reviewer: shafik. This revision rewrites FPOptionsOverride like @rjmccall suggests. There are a couple problems, i'll add inline comments in the trouble areas Repository:

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

rjmccall added inline comments. Comment at: clang/include/clang/AST/Expr.h:2280 } + FPOptionsOverride getFPFeatures(const LangOptions &LO) const { +if (UnaryOperatorBits.HasFPFeatures) I would call this one getFPOptionOverrides or getOverriddenFPOptions.

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

mibintc updated this revision to Diff 271188. mibintc added a comment. This version passes all the lit tests, i believe it's functional tho' maybe not elegant. I still need to add a test case that PCH behaves as desired, and that the floating point command line options from PCH create do not clo

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

mibintc marked an inline comment as done. mibintc added a comment. @rjmccall You suggested that the FPFeatures could be delta instead of absolute settings, I think this is approximately what you mean. This is a rough patch employing that idea, would welcome your early comments. There are 2 cla

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

mibintc created this revision. mibintc added a reviewer: rjmccall. Herald added subscribers: llvm-commits, dexonsmith. Herald added projects: clang, LLVM. mibintc marked an inline comment as done. mibintc added a comment. @rjmccall You suggested that the FPFeatures could be delta instead of absolu