sepavloff 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);
----------------
efriedma wrote:
> sepavloff wrote:
> > efriedma wrote:
> > > sepavloff wrote:
> > > > efriedma wrote:
> > > > > 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.
> > > > > 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. 
> > > > 
> > > > Could you please give any reference to this statement? I thought the 
> > > > initial state should be FE_TONEAREST, as it follows from F.8.3p1.
> > > " If no FENV_ROUND pragma is in effect, or the specified constant 
> > > rounding mode is FE_DYNAMIC, rounding is according to the mode specified 
> > > by the dynamic floating-point environment".  And all the other rules for 
> > > FENV_ROUND only apply to "an FENV_ROUND pragma establishing a mode other 
> > > than FE_DYNAMIC".  So no FENV_ROUND is equivalent to "FENV_ROUND 
> > > FENV_DYNAMIC".
> > > 
> > > The text also says, "If the FE_DYNAMIC mode is specified and FENV_ACCESS 
> > > is 'off', the translator may assume that the default rounding mode is in 
> > > effect".  But that's the same result you'd get if you didn't specify 
> > > FENV_ROUND at all: it's just combining the general rule that you're not 
> > > allowed to mess with the dynamic rounding mode outside the scope of 
> > > FENV_ACCESS, with the rule that the initial rounding mode is "nearest".
> > Thank you for the references. Indeed, default behavior specified by the 
> > standard is to use dynamic rounding. It however do not agree with the 
> > default compiler behavior - to use FE_TONEAREST. That's why we cannot make 
> > `setRoundingMode(llvm::RoundingMode::Dynamic)`. 
> > 
> > With options '-frounding-math' compiler conforms to the latest standard 
> > draft. In this case however FENV_ACCESS is not necessary. Using FE_DYNAMIC 
> > with subsequent deduction of actual rounding mode looks a fragile solution 
> > compared with setting actual mode in CurFPFeatures.
> My goal is to make the state transitions as simple as possible.  It's very 
> easy to mess up state machines.  This means as few variables as possible, in 
> as few states as possible, are involved in parsing the relevant pragmas.
> 
> > It however do not agree with the default compiler behavior - to use 
> > FE_TONEAREST.
> 
> My goal is to make the implementations of ActOnPragmaFEnvAccess and 
> ActOnPragmaFEnvRound as simple as possible.  So the question is, if we store 
> the state of FENV_ACCESS as a boolean, and the state of FENV_ROUND as a 
> rounding mode that defaults to FE_DYNAMIC, do we have enough information to 
> produce correct code?  I think we do, and you haven't given a counterexample.
> 
> I'm less concerned about how complicated it is to compute any state derived 
> from those variables.
I see your point. The patch is remade according to this goal.


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