zahiraam marked 2 inline comments as done.
zahiraam added inline comments.

================
Comment at: clang/include/clang/Basic/DiagnosticFrontendKinds.td:50-53
+def warn_eval_method_setting_via_option_in_value_unsafe_context : Warning<
+    "setting the eval method via '-ffp-eval-method' has not effect when 
numeric "
+    "results of floating-point calculations aren't value-safe.">,
+    InGroup<IncompatibleFPOpts>;
----------------
aaron.ballman wrote:
> andrew.w.kaylor wrote:
> > zahiraam wrote:
> > > aaron.ballman wrote:
> > > > Unless you have a strong reason for this to be a warning, this seems 
> > > > like a situation we should diagnose as an error with a much clearer 
> > > > message.
> > > May  be @andrew.w.kaylor would weigh in on this?
> > I was going to say that for the command line option we could just issue a 
> > warning saying that the later option overrides the earlier, but it's a bit 
> > complicated to sort out what that would mean if the eval method follows a 
> > fast-math option and it might not always be what the user intended. So, I 
> > guess I'd agree that it should be an error.
> > 
> > For the case with pragmas, the model I'd follow is the mixing of #pragma 
> > float_control(except, on) with a fast-math mode or #pragma 
> > float_control(precise, off) with a non-ignore exception mode. In both those 
> > cases we issue an error.
> > For the case with pragmas, the model I'd follow is the mixing of #pragma 
> > float_control(except, on) with a fast-math mode or #pragma 
> > float_control(precise, off) with a non-ignore exception mode. In both those 
> > cases we issue an error.
> 
> Good catch, I think that's a good approach as well.
I think i  will have the issue with the order of appearance of the options on 
the command line. 
// RUN: -freciprocal-math -mreassociate   -ffp-eval-method=source 
and
// RUN: -mreassociate -ffp-eval-method=source 

will depend on which order I will test for 
LangOpts.ApproxFunc/AllowFPReasson/AllowRecip being used or not?

The run lines above might give the same diagnostic. Unless I do something 
really complicated to check the order of the options on the command line?


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

https://reviews.llvm.org/D122155

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

Reply via email to