[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

This revision was automatically updated to reflect the committed changes. Closed by commit rGe773216f4636: [clang][patch] Add builtin __arithmetic_fence and option fprotect-parens (authored by mibintc). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

mibintc reopened this revision. mibintc added a comment. This revision is now accepted and ready to land. buildbot fails on lldb need to update this patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100118/new/ https://reviews.llvm.org/D100118

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4f1238e44d80: [clang][patch][fpenv] Add builtin __arithmetic_fence and option fprotect-parens (authored by mibintc). Repository: rG LLVM Github

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In general, I think this looks good (I did find one minor nit that removes an include, but that can be fixed as you land). There's an open question for @jansvoboda11 and it'd be

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

mibintc updated this revision to Diff 354301. mibintc added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100118/new/ https://reviews.llvm.org/D100118 Files: clang/docs/UsersManual.rst clang/include/clang/Basic/Builtins.def

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

mibintc marked 11 inline comments as done. mibintc added a comment. some inline replies. I think this is all set now. Comment at: clang/lib/Sema/SemaExpr.cpp:4026 + !E->isLValue() && + (ExprTy->isFloatingType() || (ExprTy->isComplexType( { +return

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

mibintc updated this revision to Diff 354061. mibintc marked 2 inline comments as done. mibintc added a comment. Respond to @aaron.ballman 's review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100118/new/ https://reviews.llvm.org/D100118 Files:

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

aaron.ballman added inline comments. Comment at: clang/test/AST/arithmetic-fence-builtin.c:34 + + v = (a + b); + aaron.ballman wrote: > Does the `(a + b)` still have an AST node for the `ParenExpr`? Nevermind, it won't, it'll have the `CallExpr` node. I think

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

aaron.ballman added inline comments. Comment at: clang/include/clang/Driver/Options.td:1757 + LangOpts<"ProtectParens">, DefaultFalse, + PosFlag aaron.ballman wrote: > > Should this option also be exposed to clang-cl (should it be a > > `CoreOption`)? > I don't know the

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

mibintc marked 9 inline comments as done. mibintc added a comment. A couple inline replies to go along with the updated patch Comment at: clang/include/clang/Driver/Options.td:1757 + LangOpts<"ProtectParens">, DefaultFalse, + PosFlag Should this option also be exposed to

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

mibintc updated this revision to Diff 354009. mibintc added a comment. The patch I uploaded for review yesterday wasn't correct, not sure what happened. This one looks better. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100118/new/

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

aaron.ballman added a comment. In D100118#2833864 , @mibintc wrote: > This patch addresses almost all the review comments, not yet sure about > @aaron.ballman 's question about CoreOptions FWIW, I'm fine addressing that comment in a follow-up. Also, it

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

mibintc updated this revision to Diff 353719. mibintc added a comment. Herald added subscribers: jdoerfert, hiraditya. This patch addresses almost all the review comments, not yet sure about @aaron.ballman 's question about CoreOptions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

joerg added a comment. Thanks, that's better. The clang front-end option is not directly relevant for the semantic of the intrinsic, so it would be better to explicitly specify it was not being fuseable unless the operand degenerates into a single operand. Otherwise the specification sounds

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

mibintc added a comment. Reply to @joerg proposing new wording for the option description Comment at: clang/docs/UsersManual.rst:1484 + Where unsafe floating point optimizations are enabled, this option + determines whether the optimizer honors parentheses when

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

joerg added inline comments. Comment at: clang/docs/UsersManual.rst:1484 + Where unsafe floating point optimizations are enabled, this option + determines whether the optimizer honors parentheses when floating-point + expressions are evaluated. If unsafe floating point

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

aaron.ballman added inline comments. Comment at: clang/docs/UsersManual.rst:1484 + Where unsafe floating point optimizations are enabled, this option + determines whether the optimizer honors parentheses when floating-point + expressions are evaluated. If unsafe floating

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

mibintc added a comment. question for @aaron.ballman Comment at: clang/docs/UsersManual.rst:1484 + Where unsafe floating point optimizations are enabled, this option + determines whether the optimizer honors parentheses when floating-point + expressions are evaluated.

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

aaron.ballman added reviewers: rsmith, rjmccall. aaron.ballman added inline comments. Comment at: clang/docs/UsersManual.rst:1484 + Where unsafe floating point optimizations are enabled, this option + determines whether the optimizer honors parentheses when floating-point +

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

mibintc updated this revision to Diff 349644. mibintc added a comment. I made a change to ActOnParenExpr to check that the parenthesized expression is not an lvalue before inserting the call to builtin __arithmetic_fence Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

mibintc updated this revision to Diff 348052. mibintc retitled this revision from "[clang] RFC Support new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens" to "[clang] Add support for new builtin __arithmetic_fence to control floating