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

[Melanie Blower via Phabricator via llvm-commits]

mibintc added a comment.

some inline comments for reviewers



================
Comment at: clang/include/clang/Basic/TargetInfo.h:1162
   /// the language based on the target options where applicable.
-  virtual void adjust(LangOptions &Opts);
+  virtual void adjust(DiagnosticsEngine &Diags, LangOptions &Opts);
 
----------------
There's no good place to diagnose when LangOptions and TargetOptions conflict, I see "conflict" diagnostics being emitted in clang/CodeGen when creating the func or module, which seems wrong to me. On the other hand, the "adjust" function makes a good place to do the reconciliation I think. This is the change that could be pulled out in a refactoring and committed prior to this patch. What do you think? 


================
Comment at: clang/include/clang/Driver/Options.td:4300
 defm pack_derived : BooleanFFlag<"pack-derived">, Group<gfortran_Group>;
-defm protect_parens : BooleanFFlag<"protect-parens">, Group<gfortran_Group>;
+//defm protect_parens : BooleanFFlag<"protect-parens">, Group<gfortran_Group>;
 defm range_check : BooleanFFlag<"range-check">, Group<gfortran_Group>;
----------------
@jansvoboda11 This is a gfortran option, but I don't think there's a way to allow the same option spelling for gfortran and clang. Other clang options, like -short-enum, also have been pulled out of gfortran group, and the gfortran option test is an expected-fail test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100118/new/

https://reviews.llvm.org/D100118