kbarton added a comment.

I think this looks straightforward, as long as people agree to have a separate 
CreateConstrained* version of these functions. I'm not qualified to weigh in on 
that as I don't know Clang at all and can't comment about the tradeoffs 
(although I think they have been well articulated in the discussion). From what 
I can see, that is the only thing blocking this from getting approved  (unless 
there is something else I missed while reading the discussion).

The only other comment I have is there was some very good description about the 
intention here from @uweigand, @cameron.mcinally and @andrew.w.kaylor and @kpn. 
I think it would be good if that discussion is extracted from this review and 
put somewhere (perhaps the language ref) to indicate precisely what the 
semantics are we are trying to achieve with FENV_ACCESS ON/OFF.



================
Comment at: include/llvm/IR/IRBuilder.h:228
+  /// Enable/Disable use of constrained floating point math
+  void setIsConstrainedFP(bool IsCon) { IsFPConstrained = IsCon; }
+
----------------
This is a minor quibble, but the method is setIsConstrainedFP, while the member 
is IsFPConstrained. 
I'm not sure if that is intentionally different, or an oversight. 


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

https://reviews.llvm.org/D53157



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

Reply via email to