[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Blower, Melanie via cfe-commits]

Thanks for your review. I put some quick replies below.  I'll work on an update to this. 

> -----Original Message-----
> From: Kevin P. Neal via Phabricator [mailto:reviews at reviews.llvm.org]
> Sent: Thursday, July 25, 2019 1:09 PM
> To: Blower, Melanie <melanie.blower at intel.com>; chandlerc at gmail.com
> Cc: mgorny at gentoo.org; hiraditya at msn.com; wuzish at cn.ibm.com; Wang,
> Pengfei <pengfei.wang at intel.com>; lebedev.ri at gmail.com;
> cameron.mcinally at nyu.edu; llvm-commits at lists.llvm.org; Rice, Michael P
> <michael.p.rice at intel.com>; Kaylor, Andrew <andrew.kaylor at intel.com>;
> kevin.neal at sas.com; cfe-commits at lists.llvm.org; paul.robinson at sony.com;
> peter.waller at arm.com
> Subject: [PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-
> speculation= : specify floating point behavior
> 
> kpn added a comment.
> 
> In D62731#1601310 <https://reviews.llvm.org/D62731#1601310>, @mibintc
> wrote:
> 
> > I think it would be convenient to have an "unset" setting for the different
> constrained modes, otherwise you need a boolean that says "no value was
> provided for this option".  But i'm a frontend person so I may need to have my
> attitude adjusted.
> 
> 
> What "different constrained modes"? The IRBuilder is either in constrained
> mode or it isn't. In constrained mode the exception behavior and rounding
> mode both have defaults, and those defaults can be changed individually
> without affecting the other setting. The current defaults can also be retrieved if
> you need something for a function call where you don't want to change it but
> need an argument anyway. When do you need this "no value provided" setting?
[Blower, Melanie] I was thinking it would be convenient to have null values for RoundingMode and ExceptionBehavior but I'll work on the patch more in this area
> 
> Oh, I'd check the tools/clang/CODE_OWNERS.txt file and add additional
> appropriate reviewers. Perhaps John McCall and Richard Smith? I don't know
> who has opinions on how command line options should be handled.
[Blower, Melanie] Good idea but I'm not quite ready yet
> 
> Do we want the Unix driver to be compatible with gcc? Maybe, maybe not.
> Opinions, anyone?
[Blower, Melanie] For Intel's icc compiler, we support options describing fp-model in both Linux and Windows. Our customers find it useful. I want to add it both places. Gcc doesn't support these options.
> 
> The documentation request from lebedev.ri isn't in this ticket yet.
> 
> Also, for future historical research purposes I'd cut and paste the relevant
> portions of outside web pages (like intel.com's) and post them somewhere llvm-
> ish where they are findable. This ticket, for example, is a good place. Web sites
> gets reorganized or vanish in full or in part. It's helpful to have some insulation
> from that over time. I've had to fix compiler bugs that actually were 25 years old
> before. Yes, 25 years old. Being able to do the research is very helpful.
[Blower, Melanie] good idea, I will address that, adding it to this patch description? Or inline as comments in the source file itself?  (I may be able to compete with you on oldest bug fixed...)
> 
> Oh, it may be useful to know that constrained floating point and the
> FastMathFlags are not mutually exclusive. I don't know if that matters here or
> not, but you did mention FastMathFlags.
[Blower, Melanie] yes I worry about that
> 
> 
> 
> ================
> Comment at: llvm/lib/IR/FPState.cpp:78
> +  }
> +
> +  Builder.setIsFPConstrained(IsConstrainedExcept || IsConstrainedRounding);
> ----------------
> The IRBuilder already has defaults for exception behavior and rounding. Is it a
> good idea to duplicate that knowledge here? Worse, what's here is different
> from what's in the IRBuilder. Why not ask the IRBuilder what its current setting
> is and use that?
[Blower, Melanie] I was/am/ trying to translate the fp-model command line options semantics. I will look at this again.
> 
> Would it make sense to have setters/getters, and then have a separate
> updateBuilder() method? We still don't have a good way to get the #pragma
> down to the lower levels of clang. The current, unfinished, attempt doesn't work
> for C++ templates and I'm told other cases as well. An updateBuilder() method
> could be helpful when moving from one scope to another. But keep in mind that
> if any constrained fp math is used in a function then the entire function has to be
> constrained.
[Blower, Melanie] hmm
> 
> Given that last bit, it may make more sense to have the support somewhere
> higher level than as a wrapper around the IRBuilder. Maybe in
> CodeGenFunction or CodeGenModule? I've not spent much time in clang so I'm
> not sure if that makes sense or not.
> 
> 
> Repository:
>   rL LLVM
> 
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D62731/new/
> 
> https://reviews.llvm.org/D62731
> 
>