[PATCH] D77247: Merge FloatingPointMode and FPEnv headers

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D77247#1957368 <https://reviews.llvm.org/D77247#1957368>, @sepavloff wrote:

> In D77247#1957276 <https://reviews.llvm.org/D77247#1957276>, @arsenm wrote:
>
> > In D77247#1957027 <https://reviews.llvm.org/D77247#1957027>, @arsenm wrote:
> >
> > > In D77247#1955942 <https://reviews.llvm.org/D77247#1955942>, @cameron.mcinally wrote:
> > >
> > > > The `fp::` qualifier is kind of verbose. I'm not familiar with LLVM's C++ rules, but can we replace them with `using namespace fp;`? Or can we remove the `fp` namespace completely?
> > >
> > >
> > > I’m not sure, I just merged into FPEnv and followed what it was doing.
> >
> >
> > I think llvm tends to use namespaces around enums as a substitute to have qualified enum values (which I guess is a poor substitute for enum class). I would be fine eliminating the namespace and switching the existing enums to use enum class.
>
>
> Using C++ strict enums looks a better way than namespaces.
>
> Interestingly, I am working on similar patch, that unifies rounding mode enums. But I have to move the enum `RondingMode` to `ADT/FloatingPointMode.h` because the rounding mode enum is used in APFloat and it is not a good thing to refer IR header in Support library. If you consider using `DenormalMode` in APFloat (for example to implement constant folding dependent on this mode) you might consider keeping its definition in more common place.


I think the denormal and rounding modes should be kept together, so I'm fine merging in either direction


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

https://reviews.llvm.org/D77247