efriedma added inline comments.

================
Comment at: clang/include/clang/Basic/LangOptions.h:622
     setFPContractMode(LangOptions::FPM_Off);
     setRoundingMode(static_cast<RoundingMode>(LangOptions::FPR_ToNearest));
     setFPExceptionMode(LangOptions::FPE_Ignore);
----------------
sepavloff wrote:
> efriedma wrote:
> > sepavloff wrote:
> > > efriedma wrote:
> > > > I'm suggesting this should be 
> > > > `setRoundingMode(llvm::RoundingMode::Dynamic)`.
> > > > 
> > > > If FENV_ACCESS is off, getEffectiveRoundingMode() converts that to 
> > > > "nearest".  If FENV_ACCESS is turned on, the mode is treated as 
> > > > dynamic.  This is exactly what we want.
> > > It would change semantics. In particular, it would make impossible to use 
> > > FP arithmetic in constexpr functions. An expression like `1.0 / 3.0` 
> > > cannot be evaluated with dynamic rounding mode.
> > Can we just change the relevant code in ExprConstant to call 
> > getEffectiveRoundingMode()?
> What is the advantage of using FE_DYNAMIC in the case when it is known that 
> rounding mode is FE_TONEAREST?
> 
> Probably `FENV_ROUND FE_DYNAMIC` should result in `dynamic` rounding even if 
> `FENV_ACCESS ON` is absent. Rounding mode cannot be changed in this case but 
> it can be used to inform the compiler that such function can be called in 
> environment, where rounding mode is non-default. It would prevent some 
> constant evaluations and other transformations that assume rounding mode is 
> known. Anyway the standard only allow to assume FE_TONEAREST but does not 
> require this.
> 
> In such case `getEffectiveRoundingMode` is not needed.
I really want to keep the state we store, and the state updates, as simple as 
possible; if that means making getEffectiveRoundingMode() or whatever more 
complicated, that's fine.  It's a matter of making sure we understand all the 
relevant transitions.

As far as I can tell, according to the standard, the initial state for 
FENV_ROUND is supposed to be FE_DYNAMIC, as if "#pragma STDC FENV_ROUND 
FE_DYNAMIC" were written at the beginning of every file.  If we think we need a 
new state, something like FE_DYNAMIC_INITIAL, to represent the initial state, 
we can, I guess, but I don't think the standard text requires it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126364

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

Reply via email to