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

Fri Jul 26 12:39:37 PDT 2019

kpn added a comment.

I actually don't have much of an opinion on what the command line argument form should be. It may be helpful for it to be the same as one of the commonly deployed compilers. The worst I think would be pretty close but with subtle differences. So if it can be made to work like Intel's compiler I'm fine with that. But I'm hoping that more people in the community chime in since having a consensus would be best. Personally, I'm not yet giving any final sign-offs to tickets since I don't think I've been here long enough.

As far as the rounding metadata argument, the language reference says this:

> For values other than “round.dynamic” optimization passes may assume that the actual runtime rounding mode (as defined in a target-specific manner) matches the specified rounding mode, but this is not guaranteed. Using a specific non-dynamic rounding mode which does not match the actual rounding mode at runtime results in undefined behavior.

Be aware that currently neither of the metadata arguments does anything. They get dropped when llvm reaches the SelectionDAG. And none of the optimization passes that run before that know anything about constrained intrinsics at all. This means they treat code that has them conservatively. Preserving and using that metadata in the optimization passes and getting it down and used by the MI layer is planned but hasn't happened yet. So the full set of arguments may not make sense yet, but an on/off switch for strict mode hopefully does.

================
Comment at: llvm/lib/IR/FPState.cpp:78
+  }
+
+  Builder.setIsFPConstrained(IsConstrainedExcept || IsConstrainedRounding);
----------------
mibintc wrote:
> kpn wrote:
> > 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?
> > 
> > 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.
> > 
> > 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.
> Yes I absolutely don't want to duplicate, and I will submit another version of this patch and i'll be removing fpstate*.  I do want to be able to make the fp-model options match the same behavior as icc is using.  One reason i wanted to keep track of the state within a separate object is because i was uncertain if stuff would be going on in the IR builder which would be changing the settings, for whatever reason, and i'd want to put them back into settings specified by the command line options before creating the constrained intrinsics in clang/codegen. 
> 
> let me work on this patch more, i just did a quick update to the latest before sending this up.
> 
> As far as pragmas versus templates, this is a concern.  Is there something I can read to learn more about the issue?  Pragma's are used in OpenMP so there must be a way to have pragma's interact politely with templates?  Knowing very little, I thought that the pragma would be held as a _Pragma, sort of like a function call, within the intermediate representation e.g. as opposed to a token stream from the preprocessor and I didn't think there would be a problem with templates per se. I'll check with other folks here at Intel. There was  a concern about inlining constrained intrinsics into a function because of your rule about whole function body but nobody mentioned a problem with templates.
See "https://reviews.llvm.org/D52839", "Inform AST's UnaryOperator of FENV_ACCESS". It was there that Richard Smith brought up the issue of templates. I've been prioritizing work on the llvm end and haven't had a chance to get to understand how the relevant parts on the clang side work myself. My hope is that maybe command line arguments can go in to enable strict FP on a compilation-wide basis, with support for the #pragma coming later. But I don't know if it will work out that way.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731