sepavloff added a comment. In D126364#3546173 <https://reviews.llvm.org/D126364#3546173>, @efriedma wrote:
>> setRoundingMode definitely should not call getAllowFEnvAccess() and it does >> not > > Yes, it does? > > // C2x: 7.6.2p3 If the FE_DYNAMIC mode is specified and FENV_ACCESS is > "off", > // the translator may assume that the default rounding mode is in effect. > if (FPR == llvm::RoundingMode::Dynamic && > !CurFPFeatures.getAllowFEnvAccess() && > CurFPFeatures.getFPExceptionMode() == LangOptions::FPE_Ignore) > FPR = llvm::RoundingMode::NearestTiesToEven; Sorry for my confusion, I meant setAllowFEnvAccess(). >> As for ActOnPragmaFEnvAccess, it make sense to set the most general mode. > > Shouldn't the default mode just be "dynamic"? The fact that we emit > non-strict floating-point ops if FENV_ACCESS is disabled is an implementation > detail. I don't see the point of switching the rounding mode between > "Dynamic" and "NearestTiesToEven" depending on whether FENV_ACCESS is > enabled. Making the required state transitions complicated like this just > makes the code harder to understand. The standard says that the translator may assume that the default rounding mode is in effect so it does not require to make such assumption, and we can use the most profitable way. The advantages of using default rounding mode are: 1. In this case the mode has concrete value. Constant evaluator can evaluate expressions like `1.0/3.0`, which is not allowed if rounding is dynamic, and the expression is not constexpr anymore. 2. On targets that support static rounding mode (like RISCV) dynamic and constant rounding modes may be different and the behavior changes if default mode is replaced by dynamic. In these cases choosing dynamic mode is not an implementation details, it has user-visible effect and the behavior should be chosen most close to the standard. It seems that `#pragma STDC FENV_ROUND FE_DYNAMIC` without `#pragma STDC FENV_ACCESS ON` is more like an error, because there is no legitimate way to set rounding mode in this case. 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
