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