mibintc marked 3 inline comments as done.
mibintc added inline comments.

================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:126
+  case LangOptions::FPM_Precise:
+  case LangOptions::FPM_Fast:
+    break;
----------------
kpn wrote:
> Wait, so "fast" and "precise" are the same thing? That doesn't sound like 
> where the documentation you put in the ticket says "the compiler preserves 
> the source expression ordering and rounding properties of floating-point".
> 
> (Yes, I saw below where "fast" turns on the fast math flags but "precise" 
> doesn't. That doesn't affect my point here.)
"precise" doesn't necessitate the use of Constrained Intrinsics, And likewise 
for "fast".   The words "compiler preserves the source expression ordering" 
were copied from the msdn documentation for /fp:precise as you explained it 
would be useful to have the msdn documentation for the option in case it goes 
offline in, say, 30 years.  The ICL Intel compiler also provides equivalent 
floating point options. The Intel documentation for precise is phrased 
differently "Disables optimizations that are not value-safe on floating-point 
data."  

fp-model=precise should enable contractions, if that's not true at default (I 
mean, clang -c) then this patch is missing that.

fp-model=fast is the same as requesting ffast-math 


================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3052
+    else
+      llvm_unreachable("invalid -fp-model setting");
+  }
----------------
kpn wrote:
> Shouldn't this be a call to Diags.Report() like in the code just above it and 
> below? Same question for _some_ other uses of llvm_unreachable().
I put it in as unreachable because the clang driver shouldn't build this 
combination, but that's a good point I can just switch it to match the other 
code in this function, thanks.


================
Comment at: clang/test/CodeGen/fpconstrained.c:22
+  // STRICTNOEXCEPT: llvm.experimental.constrained.fadd.f32(float %0, float 
%1, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+  // PRECISE: fadd float
+  // FAST: fadd fast
----------------
kpn wrote:
> This is another case of "fast" and "precise" doing the same thing. If we're 
> using the regular fadd then it cannot be that "the compiler preserves the 
> source expression ordering and rounding properties of floating-point".
I need an fp wizard to address this point, @andrew.w.kaylor ??

The msdn documentation says that strict and precise both preserve ... 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

Reply via email to